C++: Composition Interface

Question

So I've spent some time thinking about this and been all over google looking for a 'solution' (the solution is most likely a matter of preference, but I'm unsure about this). Below is the problem I have, but the problem can be applied to a lot of situations regarding composition.

I have a class, Colour, containing the members of red, green, blue and alpha. Each of these have a set and get member funtion. Simple enough.

class Colour
{
    public:
        float getRed();
        void setRed(float);
        float getGreen();
        void setGreen(float);
        ...
    private:
        float red;
        float green;
        ...
};

I will use this class within other classes to dictate their colour, for example (omitting the constructor and destructor for brevity):

class ColourableObject
{
    private:
        Colour colour;
};

Now my problem is this: How would this ColourableObject class best access this Colour object? Would it be best to retreive the Colour object to access its member functions directly, like so:

class ColourableObject
{
    public:
        Colour& getColour();
    private:
        Colour colour;
};

Or, would it be better to give the ColourableObject class its own set and get functions for colour, in which act upon the Colour object, like so:

class ColourableObject
{
    public:
        float getRed();
        void setRed(float);
        ...
    private:
        Colour colour;
};

To me, the former is the most logical as it would save a lot of trouble adding functionality to each class that requires this Colour object by simply acting directly upon this Colour object.

However, isn't the latter more susceptible for changes to the ColourableObject class? Not to mention that colourableObject.getColour().setRed(x) doesn't seem too natural to me, as the get and set clash with each other.

I probably have the wrong approach entirely. I'm relatively new to C++, so I'm willing to learn! So the question is, should I use the former method, latter method, or an entirely different method?

Answer 1

Not to directly answer your question, but I would get rid of all the setters (usually a bad sign to have setters in any case), and use a constructor:

Color c = Color( 123, 17, 77 );

where the three constructor parameters are the RGB values.

Answer 2

I probably would recreate your class in case you may need to add other colors like Orange, Purple, Cyan, or Crayola's new Smooky Applewood!

Sort of like a base color class and act upon that base class by introducing a new color. That way it does not matter which color you are dealing with. It's all a black box. Also that will answer your second question as because it doesn't matter which color it is you do not have to redefine your setColor and getColor name methods. They would not need to know or care what color you are referring to.

I think Code Complete (the book) had a section to be a bit leary about seeing classes that have too many get/set methods. It usually boils down to the wrong way of doing things.

Answer 3

Your use of getters and setters in this case don't make a lot of sense. You can just as well get away with making the float members of Colour public. Unless you need to do validation or manipulation of more than one private member, just go ahead and make those public, and create a constructor to initialize Colour apprpriately.

As to the ColourableObject, the question you should be asking is: Will other, unrelated classes need access to the object's Colour member? Will they need to make changes to it? If the answer to either of those is "no," I would say that you should not have any kind of getter or setter on that object at all. Otherwise, again, unless you need to do validation or additional state changes, just make the Colour public.

Answer 4

The DRY principle supports your first option, providing access to the Colour object.

Furthermore, you may wish to change

    Colour& getColour();

to

    const Colour& getColour();
    void setColour( const Colour& );

...as that will ensure that your ColourableObject will always know when its colour has changed.

Answer 5

Personally, I'd prefer dropping the "get" part, in the case of Colorable's member:

colorableObject.Color().setRed(1.0f);

This is, as you said, entirely personal preference (as is the spelling! :) but this way, to me it looks like a "property", rather than a getter/setter method.