2010-01-26 15 views
6
class Room{ 
    public: 
    void ColorRoom(){}; 
}; 

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

1) Zimmer kann nicht ohne ein Haus existieren, ein Haus "hat ein" Zimmer. (Zusammensetzung)
2) Eine andere Möglichkeit, den Raum zu färben, wäre eine Methode in House, die den ColorRoom in der Room-Methode aufruft, aber dann ist das mehr Delegierung. (Ich möchte dies vermeiden)

Die einzige Möglichkeit, die ich sehe, ist die oben genannte, aber sieht aus wie die Rückkehr Verweis auf private Mitglied bricht OOP. Ist das ein gutes Design?Bricht reine Zusammensetzung OOP-Konzepte?

Antwort

4

Insgesamt sind Sie gut, da House Elementvariable m_room von sich selbst erstellt - es erfordert keinen Verbraucher, nach der Instanziierung etwas aufzurufen. Dies folgt dem Muster, dass ein Element unmittelbar nach der Instanziierung verwendbar ist (es erfordert keine speziellen Aktionen wie das Einstellen eines Raums oder was auch immer).

ich einige kleinere nit-pickings haben:

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; 
}; 
+0

Das funktioniert gut, und für eine kurze Erklärung, siehe meine Antwort. –

+0

Beachten Sie, dass 'virtual' sich auf * overriding * bezieht und nicht auf * overloading *. – Dario

+0

Können Sie eine Funktion überschreiben, ohne zu erben? Zweitens, wenn der Benutzer den Raum ändern möchte, sieht es so aus, als müssten sie eine Methode von House aufrufen, die wiederum die Raummethode aufrufen würde, ähnlich wie bei der Delegation. –

2

Der Betrieb auf dem Zimmer geschieht, und nicht das Haus. Wenn Sie einen unveränderlichen Bezug zum Raum durch das Haus zurückgeben können, der dann bearbeitet werden kann, brechen Sie OOP nicht.

5

Die Sache ist, dass Sie nicht ausdrücklich Ihr privates Mitglied freilegen. Ihre API zeigt nur eine Methode an, um einen Raum zu erhalten, und der Kunde weiß nicht, ob House diesen Raum erstellt, etwas zurückgibt, das sich in einem privaten Bereich befindet, oder den Raum von einem Web-Service bezieht. Das ist solide OO.

+2

Dies ist sehr auf den Punkt, aber bitte beachten Sie, es ist in der Regel schlechtes Design, um externe Kräfte zu erlauben, Ihren Objektstatus ohne Validierung zu ändern. –

4

Ich mag NickLarsen Antwort, aber ich habe nur etwas hinzufügen:

Sie ein privates Feld eines Objekts nicht zulassen, außerhalb des Objekts geändert werden. Wenn Sie die Room Objektverwendung ändern müssen. Das ist, wenn Room eine SetFloorColor(Color _color); Methode hat, dann sollten Sie setzen in House ein Anruf

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

Obwohl ich NickLarsen Antwort mag, würde ich eine andere Sache hinweisen: Zimmer nicht Farbe (oder Farbe) selbst. Diese Aktion wird normalerweise von einem Maler ausgeführt, der offensichtlich kein Mitglied eines Raumes ist. Jetzt könnte ein Maler das ganze Haus färben oder der Maler könnte nur an einem Raum arbeiten.

Ich würde in diesem Fall vorschlagen, dass der Raum eine Farbeigenschaft hat und der Akt des Änderns dieser Farbe extern von einem anderen Objekt behandelt wird.

Diese Idee würde vorschlagen, dass die Color-Eigenschaft eine öffentliche Eigenschaft sein muss und dass Sie einen Verweis auf den zu ändernden Raum an das Painter-Objekt übergeben würden.

0

Das Freilegen von privaten Elementen reduziert die Kohäsion und erhöht die Kopplung. Im Allgemeinen sollten Sie von Code wie folgt verhindern:

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

Dieser Code ist weniger wartbar und verletzt Law of Demeter.