search

cecilianor / Qt-thesis

MIT License
9 stars 4 forks source link

Review/unit test refactor #85

Closed RafaelPalomar closed 7 months ago

RafaelPalomar commented 7 months ago

@cecilianor, here you have some suggestions after reading your post and checking your testing code (I have only checked the Evaluator tests).

1) There seems to be a small spelling mistake (feture -> feature). I have replace these. 2) I suggest a slightly different approach for the use of member variable across tests. In essence, I don't see any risks of using member variables across tests, provided that these variables are initialized properly (on init functions) and kept read-only. In this PR I propose wrapping the expresssionsObject variable in a function that returns the const form of it, so it is effectively constant. 3) I made the QFile file member variable local to the init function as it is not used anywhere else.

You can apply 2) and 3) across your tests.

cecilianor commented 7 months ago

Thank you, @RafaelPalomar ! I am approving this and working with these changes going forward.