Does pure composition break OOP concepts ?

Question

class Room{
  public:
    void ColorRoom(){};
};

class House{
  public:
    Room* GetRoom(){return &m_room;}
  private:
    Room m_room;
};

1) Room cannot exist without a house, an house "has a" room. (composition)
2) Another way to color room would be to have a method in House that would invoke the ColorRoom in Room method but then this is more of delegation. (i want to avoid this)

The only way I see is the one above but looks like returning reference to private member is breaking OOP. Is this a good design ?

Answer 1

Overall, you're good, since House creates member variable m_room by itself -- it does not require a consumer to call something after instantiation. This follows the pattern that an item is usable immediately after instantiation (it does not require special actions like setting a room or whatever).

I do have some minor nit-pickings:

class Room
{
public:
    // virtual method to allow overriding
    virtual void ColorRoom(){};
};

class House
{
public:
    // Returning a non-const pointer in C++ is typically a bad smell.
    const Room& Room() const { return m_room; }
    // Allow for assignment and manipulating room, but breaks const-ness
    Room& Room() { return m_room; }
    // Facade method for houses with more than one room
    // You can forward parameters or come up with room-painting schemes, but you should
    // not require that a House has a Room called Room().
    virtual void ColorAllRooms()
    {
        m_room.ColorRoom();
    }
private:
    Room m_room;
};

Answer 2

The operation is happening on the room, and not the house. If you can return an immutable reference to the room through the house which can then be operated on, you are not breaking OOP.

Answer 3

The thing is that you're not explicitly exposing your private member. Your API just shows a method to get a room, and the consumer doesn't know if House is creating that room, returning something held in a private field, or getting the room from a web service. That's solid OO.

Answer 4

I like NickLarsen's answer but I have one thing to add:

Don't let a private field of an object be changed outside of that object. If you must change the Room object use delegation. That is if Room has a SetFloorColor(Color _color); method then you should put into House a call to

SetRoomFloorColor(Color _color){ m_room.SetFloorColor( _color ); }

Answer 5

Although I like NickLarsen's answer, I would point out one other thing: Rooms don't color (or paint) themselves. That action is typically done by a Painter, which is obviously not a member of a room. Now a painter could color the entire house or the painter could work on just one room.

I would suggest in this case that the room has a color property and the act of changing that color is handled externally by another object.

This idea would suggest that the Color property must be a public property and that you would pass a reference to the room to be changed to the Painter object.

Answer 6

Exposing private members reduses cohesion and increases coupling. Generally, you should prevent from code like this:

selection.getRecorder().getLocation().getTimeZone(); 

This code is less maintainable and violates Law of Demeter.