views:

323

answers:

5

I have a base Color class that looks something like this. The class is designed to be immutable, so as a result has final modifiers and no setters:

public class Color
{
    public static Color BLACK = new Color(0, 0, 0);
    public static Color RED = new Color(255, 0, 0);
    //...
    public static Color WHITE = new Color(255, 255, 255);

    protected final int _r;
    protected final int _g;
    protected final int _b;

    public Color(int r, int b, int g)
    {
     _r = normalize(r);
     _g = normalize(g);
     _b = normalize(b);
    }

    protected Color()
    {

    }

    protected int normalize(int val)
    {
     return val & 0xFF;
    }
    // getters not shown for simplicity
}

Derived from this class is a ColorHSL class that in addition to providing the Color class' getters, is contructed with hue, saturation, and luminosity. This is where things stop working.

The constructor of ColorHSL needs to do some calculations, then set the values of _r, _b, and _g. But the super constructor has to be called before any calculations are made. So the parameterless Color() constructor was introduced, allowing the final _r, _b, and _g to be set later on. However, neither the parameterless constructor or the setting (for the first time, within the constructor of ColorHSL) are accepted by the Java compiler.

Is there a way around this issue, or do I have to remove the final modifier from _r, _b, and _g?


Edit:

In the end, I went for a base abstract Color class, containing both RGB and HSL data. The base class:

public abstract class Color
{
    public static Color WHITE = new ColorRGB(255, 255, 255);
    public static Color BLACK = new ColorRGB(0, 0, 0);
    public static Color RED = new ColorRGB(255, 0, 0);
    public static Color GREEN = new ColorRGB(0, 255, 0);
    public static Color BLUE = new ColorRGB(0, 0, 255);
    public static Color YELLOW = new ColorRGB(255, 255, 0);
    public static Color MAGENTA = new ColorRGB(255, 0, 255);
    public static Color CYAN = new ColorRGB(0, 255, 255);

    public static final class RGBHelper
    {
     private final int _r;
     private final int _g;
     private final int _b;

     public RGBHelper(int r, int g, int b)
     {
      _r = r & 0xFF;
      _g = g & 0xFF;
      _b = b & 0xFF;
     }

     public int getR()
     {
      return _r;
     }

     public int getG()
     {
      return _g;
     }

     public int getB()
     {
      return _b;
     }
    }

    public final static class HSLHelper
    {
     private final double _hue;
     private final double _sat;
     private final double _lum;

     public HSLHelper(double hue, double sat, double lum)
     {
      //Calculations unimportant to the question - initialises the class
     }

     public double getHue()
     {
      return _hue;
     }

     public double getSat()
     {
      return _sat;
     }

     public double getLum()
     {
      return _lum;
     }
    }

    protected HSLHelper HSLValues = null;
    protected RGBHelper RGBValues = null;

    protected static HSLHelper RGBToHSL(RGBHelper rgb)
    {
     //Calculations unimportant to the question
     return new HSLHelper(hue, sat, lum);
    }

    protected static RGBHelper HSLToRGB(HSLHelper hsl)
    {
     //Calculations unimportant to the question
     return new RGBHelper(r,g,b)
    }

    public HSLHelper getHSL()
    {
     if(HSLValues == null)
     {
      HSLValues = RGBToHSL(RGBValues);
     }
     return HSLValues;
    }

    public RGBHelper getRGB()
    {
     if(RGBValues == null)
     {
      RGBValues = HSLToRGB(HSLValues);
     }
     return RGBValues;
    }
}

The classes of RGBColor and HSLColor then derive from Color, implementing a simple constructor that initializes the RGBValues and HSLValues members. (Yes, I know that the base class if-ily contains a static instance of a derived class)

public class ColorRGB extends Color
{
    public ColorRGB(int r, int g, int b)
    {
     RGBValues = new RGBHelper(r,g,b);
    }
}

public class ColorHSL extends Color
{
    public ColorHSL(double hue, double sat, double lum)
    {
     HSLValues = new HSLHelper(hue,sat,lum);
    }
}
+4  A: 

A final variable must be assigned by the time the declaring type's constructor has been completed. Therefore, you can not assign super's final fields in the subclass.

You could, however, do the conversion in a static factory method in the subclass:

class HSLColor {
    private HSLColor(int r, int g, int b) { super(r,g,b);}

    static HSLColor create(int h, int s, int l) {
        // conversion code here
        return new HSLColor(r,g,b);
    }
}
meriton
+3  A: 

As far as I know the only way to get this done would be to nest the call of the super constructor with the function to calculate r, g, and b:

super(calculateRGB(...))

So you might consider adding a constructor to Color that takes RGB values as an array.

__roland__
+1 beat me to it...
hjhill
+1  A: 

You can't have an abstract constructor in Java, so unless you can put all the calculation into the call to super you can't do what you'd like to do with the current design as a final variable must be assigned by the time the declaring constructor has completed.

An alternative would be to factor out some methods for the creation of Color objects (a factory pattern) which takes the arguments, does the calculations external to the constructor and then you can can call super() as the first argument.

For example - if you have the following on your Color class

public Color static createHSLColor(int h, int s, int v) {
   // All the calculation here

   return new ColorHSL(h,s,v);
}

You could then make the constructors not public (maybe protected) and then the only way the objects can be created is via your factory methods.

Jeff Foster
This is a little bit misleading - better use `return new Color(r, g, b)` (after calculating the RGB values of course).
hjhill
Not sure what you mean? The original question involved a subclass of Color. If this were just one class, then there'd be no problem.
Jeff Foster
+1  A: 

One thing you could do is to have a constructor parameter which represents the calculator. So for example:

public Color(int r, int g, int b, Calc calc) {
   _r = calc.normalize(r);
   _g = calc.normalize(g);
   _b = calc.normalize(b);
}

This may possibly remove the need for a subclass completely. You could declare constructors:

public Color(int r, int g, int b) { 
  this(r,g,b, defaultCalc);
}

Or even provide static style constructors:

public static Color hslColor(int r, int g, int b) {
    return new Color(r,g,b, hslCalc);
}
oxbow_lakes
+2  A: 

Yes, I can see how super(calculateRGB(...)) - but it looks as though you gain practically nothing from inheritance here. I'd just use a common interface. Aren't RGB and HSV just two different, interchangeable colour models?

I think there's an object-oriented analysis problem here behind the Java problem. Why are you using inheritance?

If all you need to do is to manipulate a Color interchangeably, you might find that you don't benefit at all from inheritance (and it just creates an overhead of mapping HSV back into RGB for the superclass)... If you just want interchangeable colour models, consider using a Color interface rather than inheriting from RGB.

Without seeing what you're going to actually use the Colour objects for, it's hard to suggest a better design. Right now it looks as though the cost of inheritance (colour model conversion in a call to a super constructor) will outweigh the sole benefit of reusing normalize.

cartoonfox
There are more benefits than normalize: the getter functions (and equals) will also be inherited. Also, a set of constants are inherited
Eric
If you showed us more of the model that Color fits into, we could probably come up with better ideas...
cartoonfox