Vincent van Ravesteijn <v.f.vanraveste...@tudelft.nl> writes:
> Herewith I'd like to propose a different solution. I'd like to hear
> whether this is a reasonable solution or not.
I am not sure that I prefer it to my solution, but I could live with it.
> I created a new class ColorCode (and renamed the old enum to
> something else).
Why not Color?
> This class has for now one extra option, namely the possibility to
> define two different colors than can be merged. This is only done
> when ColorCode.type == MERGECOLORS. The largest part of this class
> is administration and so forth..
You could remove this type and set mergecolor to Color_ignore when no
merging is required (would be the default)
>
> Defining the new colors for change tracking is now done in two lines
> in Changes.cpp :
> + color.type = ColorCode::MERGECOLORS;
> + color.mergeColor_ = Color_white;
Please avoid public variables with name ending in '_'.
> This is basically all extra code and this can easily be used for the
> other purposes, e.g. merging the selection color with the yellow
> background color of a note.. etc.
OK.
> A drawback of this is that everywhere a ColorCode is passed, we end up
> with objects in stead of an enum-value, and we have a lot of implicit
> constructors.
I am not sure how bad it is in practice.
> Index: src/ColorCode.h
> ===================================================================
> --- src/ColorCode.h (revision 27975)
> +++ src/ColorCode.h (working copy)
> @@ -10,10 +10,12 @@
> #ifndef COLOR_CODE_H
> #define COLOR_CODE_H
>
> +#include <sstream>
I suspect andre will not like this one.
> + bool operator!=(ColorCode const & color) const
> + {
> + return (baseColor_ != color.baseColor_);
> + }
Not needed when == exists.
> + bool operator<(ColorCode const & color) const
> + {
> + return (baseColor_ < color.baseColor_);
> + }
> +
> + bool operator<=(ColorCode const & color) const
> + {
> + return (baseColor_ < color.baseColor_)
> + || (baseColor_ == color.baseColor_);
> + }
I think only one of those is needed, but never remember which one...
JMarc
