Code review comment for lp:~mc.../inkscape/SelContainer

Revision history for this message
Krzysztof Kosinski (tweenk) wrote :

Some more comments.

1. Please follow the coding style.
https://inkscape.org/en/develop/coding-style/

2. I don't really understand what you mean by "introducing iteration over boost objects".

3. While simply replacing GSList with vectors is an obvious improvement, I think the correct approach is to make a new data structure, that will encapsulate what we mean by a selection of objects (a list of unique items none of which is a descendant of any other), but which will not be tied to the GUI concept of selection. One could use Boost MultiIndex for this. Then you would return this structure (or references to it) instead of std::vector. The new structure could be a template called SelectionSet, with typedefs SelectionSet<SPObject> = ObjectSet and SelectionSet<SPItem> = ItemSet. Still, I would not be opposed to incorporating the simple GSList -> vector change first and introducing selection sets later.

« Back to merge proposal