C++: Composition Interface

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 function. 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 trou开发者_如何学Cble 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?

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.

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.

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.

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.

Ultimately, the answer depends on how you do (or do not) want to restrict access to the embedded Colour object.

In your first choice...

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

...you're really not restricting access at all. If a user can get at the private member via getColour(), what's the point of making it private? You might as well skip the intermediate step and just do this:

class ColourableObject
{
public:
    Colour colour;
};

When doing it this way, a person can just refer directly to the data member and all of its functions, such as C.colour.getRed(); (assuming C is a ColourableObject)

Now, say you want to restrict the end user in some way -- he can't set green or blue, but you'll allow him to set red. In that case you might want to use your second choice:

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

This is because a user of a ColourableObject will only have access to your public functions, and not the underlying private member. This rationale also holds true if you want to have some intermediary step between the end user and the color selection. For example, the following:

class ColourableObject
{
public:
    void setMood(enum Mood);
private:
    Colour colour;
};

void ColourableObject::setMood(enum Mood)
{
   if(Mood == HAPPY) colour.setRed(3);
   if(Mood == SAD)  colour.setBlue(11);
   ...
}

Lastly, you could do a hybrid that keeps the colour data member public, but also adds additional accessors to that data member.

Summary:

• First method:

  • Pro's: Interface to Colour carries through.
  • Con's: Makes data member public.

• Second method:

  • Pro's: Encapsulates data. Future changes to Colour's interface would only break your code, not the end user's.
  • Con's: You have to re-implement any part of Colour's interface that you want the end user to be able to use.

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.