New repo in kdereview: kalk

[Ivan Čukić]

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ć
ivan at cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232  07AE 01C6 CE2B FF04 1C12