-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate ViewModel layer to C# #1894
Comments
@tian-lt, @hanzhang54, or @guominrui I'd love your opinion on this proposal 😄 |
Hi @mcooley, the plan looks great.
|
Yes, I think we should move unit converter out of CalcManager as part of this work. Unit converter is already mostly separate from the rest of CalcManager--the only common file is Command.h--and I don't think there's any reason to keep the implementation in C++. For the parts of CalcViewModel which act as a bridge to CalcManager--StandardCalculatorViewModel, History(Item)ViewModel, MemoryItemViewModel, and ExpressionCommand(De)Serializer--I agree that we need to redesign the interface between the ViewModel layer and the "engine" layer as described in #1545. Until we do that work, I think we should leave these classes as they are. We can track complete removal of the CalcViewModel project as part of #1545.
Yes, we should create a separate project to test CalcEngine in isolation. I think we can track this as a separate issue. |
In 2021, the view layer of the Calculator codebase was ported to C#. I propose that we continue this work and migrate the ViewModel layer to C#.
General reasons for migrating to C# are discussed in #893. I expect that we would see a few benefits specifically from migrating the ViewModel layer:
This work is best done incrementally. Here is how I would break up the work:
We should avoid modifying the infinite-precision calc engine as part of this work. StandardCalculatorViewModel, and related classes which call into the engine, should remain in C++ for now.
We should port the code as-is, and postpone major refactoring until after the code has been ported.
We should measure the app startup latency after these changes. If these changes cause startup to get slower, we should revert the changes or optimize code to eliminate the regression.
I would like to work on this in my free time, but I'd welcome help from other contributors.
The text was updated successfully, but these errors were encountered: