Le 14 janv. 09 à 23:19, Vincent van Ravesteijn a écrit :
Done. The nice thing is that we can now also make a new color: Color_deletedtextmodifier. This can be set by the user to specify the degree of lightening or darkening of the color. It would be nice if the user can also put this to Color_ignore to indicate that there shouldn't be any merging.

At least we can set it to the same color. But you are right that in general it would be nice to be able to set a color to
be the same as another one.

+       bool operator!=(ColorCode const & color) const
+       {
+               return (baseColor_ != color.baseColor_);
+       }
Not needed when == exists.
I really can't compile when this isn't defined.. am I doing something wrong ? The same for all other operators.

I thought it was done by some templates, but now I cannot find them. Forget that for now.

The following patches are attached:

I am not sure how you attached your patches, but Mail.app show them as one big patch. This is a bit annoying.

The patch is OK in general, but I have a few comments below.

+Color::Color(ColorCode base_color) : mergeColor(Color_ignore)
+{
+       baseColor = base_color;
+}

You can do AFAIK

Color::Color(ColorCode base_color) : baseColor(base_color), mergeColor (Color_ignore)
{
}

If you add a default value for base_color, you won't need the Color() constructor.

+Color::Color(int base_color) : mergeColor(Color_ignore)
+{
+       baseColor = static_cast<ColorCode>(base_color);
+}

Where is this needed?

+bool Color::operator==(Color const & color) const
+{
+       return (baseColor == color.baseColor);
+}

The parenthesis are not needed (and in the others below)

+bool Color::operator<=(Color const & color) const
+{
+       return (baseColor < color.baseColor)
+               || (baseColor == color.baseColor);
+}

why not baseColor <= color.baseColor ?

+std::ostream & operator<<(std::ostream & os, Color color)
+{
+       os << color.baseColor;
+       return os;
+}

Would it be possible to write the name of the color instead (and of the mergecolor also, maybe?)

JMarc

Reply via email to