D11815: Refactor the Solver class

Fabian Kosmale noreply at phabricator.kde.org
Sun Apr 1 18:56:43 UTC 2018


fabiank added a comment.


  Anything I can do to convince you that templates are awesome, and we should use them ;-)? My main argument is that this improves type safety and allows to catch off by one errors when indexing the arrays (and it enables iteration with range based for, which I haven't implemented yet, though). It also helped me catch that Q_ASSERT weirdness in D11814 <https://phabricator.kde.org/D11814>, as with the std::array, that assignment became illegal.
  
  However, if you think that the template makes the code more complicated than it was before, I'm willing to remove that part of the change set – the goal of my changes is to make the code easier to maintain after all.

INLINE COMMENTS

> aacid wrote in patsolve.cpp:986
> Why do you need all this lines?

I assume you mean all the template class<number> lines? This is mostly about compile times. This file contains the implementation of multiple templated methods, and those must be instantiated at some point, else we'll get an error at link time. There are generally 2 possible ways to achieve this:

1. This file gets included together with its header file, and the method definitions are visible to its users when the template is instantiated, so everything is alright.
2. The way I've done it: The method definitions are only visible here, and for each possible user, we instantiate the template. Then at link time, the compiler has emitted the necessary code and the linker can find it. Drawback: Those lines look weird to uninitiated, as you've just demonstrated. Pro: The code doesn't get included multiple times, and when 2 Solver subclasses have the same number of piles, the code gets only generated once. (With the 1st approach, the compiler would create the code once per translation unit, and at link time it would be merged.)

I'm fine with either changing the approach to 1, or adding some documentation to make the code more understandable.

REPOSITORY
  R410 KPatience

REVISION DETAIL
  https://phabricator.kde.org/D11815

To: fabiank, #kde_games
Cc: aacid, #kde_games
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20180401/9513a624/attachment.html>


More information about the kde-games-devel mailing list