Hi all, Cool idea for the project. I went through the C++ part of the code, and have a few suggestions.
> > Please reconsider your decision. > https://blog.acolyer.org/2020/10/02/toward-an-api-for-the-real-numbers/ +1 for the plea of not reinventing numerics however fun it is to play with flex/yacc. For the actual code review: historymanager.cpp: Item 1: if (file.exists()) { file.open(QIODevice::ReadOnly); This should be: if (file.open(QIODevice::ReadOnly); The fact that a file exists does not mean open will succeed. Item 2: This: for (const auto &record : qAsConst(array)) { historyList.append(record.toString()); } should be prefixed with: historyList.reserve(array.count()); Item 3: There is no need to call file.close() as its destructor will close it. Item 4: Instead of auto array and qAsConst(array), just declare array to be const: const auto array = ... or const auto& array = ... Item 5: this->save() can be only save(). It is not common to write this-> in C++ unless there is a strong reason to do so in a particular case. Item 6: QList<QString> historyList; -> QList<QString> m_historyList; inputmanager.cpp: Item 1: Great to see a correct C++ singleton :) Item 2: You can use std::deque instead of std::stack. You'll get .clear() operation. It will be more efficient than = {}; unitmodel.cpp: Item 1: Similar to above, when you know the number of elements a collection will have, call reserve(size) before you start appending (this is for m_units). BTW, these for loops can be replaced with std::transform and std::back_inserter, but I don't want to go overload the review. ;) Cheers, Ivan -- dr Ivan Čukić i...@cukic.co, https://cukic.co/ gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232 07AE 01C6 CE2B FF04 1C12
