Richtiges Design, um die Verwendung von dynamic_cast zu vermeiden?

9

Nach einigen Recherchen kann ich anscheinend kein einfaches Beispiel finden, um ein Problem zu lösen, auf das ich häufig stoße.

Angenommen, ich möchte eine kleine Anwendung erstellen Square, in der ich s, Circles und andere Formen erstellen , sie auf einem Bildschirm anzeigen, ihre Eigenschaften nach Auswahl ändern und dann alle ihre Umfänge berechnen kann.

Ich würde die Modellklasse so machen:

class AbstractShape
{
public :
    typedef enum{
        SQUARE = 0,
        CIRCLE,
    } SHAPE_TYPE;

    AbstractShape(SHAPE_TYPE type):m_type(type){}
    virtual ~AbstractShape();

    virtual float computePerimeter() const = 0;

    SHAPE_TYPE getType() const{return m_type;}
protected :
    const SHAPE_TYPE  m_type;
};

class Square : public AbstractShape
{
public:
    Square():AbstractShape(SQUARE){}
    ~Square();

    void setWidth(float w){m_width = w;}
    float getWidth() const{return m_width;}

    float computePerimeter() const{
        return m_width*4;
    }

private :
    float m_width;
};

class Circle : public AbstractShape
{
public:
    Circle():AbstractShape(CIRCLE){}
    ~Circle();

    void setRadius(float w){m_radius = w;}
    float getRadius() const{return m_radius;}

    float computePerimeter() const{
        return 2*M_PI*m_radius;
    }

private :
    float m_radius;
};

(Stellen Sie sich vor, ich habe mehr Klassen von Formen: Dreiecke, Sechsecke, jedes Mal ihre Proprers-Variablen und zugehörige Getter und Setter. Die Probleme, mit denen ich konfrontiert war, hatten 8 Unterklassen, aber für das Beispiel habe ich bei 2 angehalten.)

Ich habe jetzt eine ShapeManager, die alle Formen in einem Array instanziiert und speichert:

class ShapeManager
{
public:
    ShapeManager();
    ~ShapeManager();

    void addShape(AbstractShape* shape){
        m_shapes.push_back(shape);
    }

    float computeShapePerimeter(int shapeIndex){
        return m_shapes[shapeIndex]->computePerimeter();
    }


private :
    std::vector<AbstractShape*> m_shapes;
};

Schließlich habe ich eine Ansicht mit Spinboxen, um jeden Parameter für jeden Formtyp zu ändern. Wenn ich beispielsweise ein Quadrat auf dem Bildschirm auswähle, zeigt das Parameter-Widget nur Square-bezogene Parameter an (dank AbstractShape::getType()) und schlägt vor, die Breite des Quadrats zu ändern. Dazu benötige ich eine Funktion, mit der ich die Breite ändern kann ShapeManager, und so mache ich das:

void ShapeManager::changeSquareWidth(int shapeIndex, float width){
   Square* square = dynamic_cast<Square*>(m_shapes[shapeIndex]);
   assert(square);
   square->setWidth(width);
}

Gibt es ein besseres Design, das mich daran hindert, das dynamic_castGetter / Setter-Paar ShapeManagerfür jede Unterklassenvariable zu verwenden und zu implementieren ? Ich habe bereits versucht, eine Vorlage zu verwenden, bin jedoch fehlgeschlagen .


Das Problem , das ich bin Bewurf ist nicht wirklich mit Formen , aber mit unterschiedlichem Jobs für einen 3D - Drucker (zB: PrintPatternInZoneJob, TakePhotoOfZone, etc.) mit AbstractJobals ihre Basisklasse. Die virtuelle Methode ist execute()und nicht getPerimeter(). Das einzige Mal, dass ich konkrete Verwendung verwenden muss, ist das Ausfüllen der spezifischen Informationen, die ein Job benötigt :

  • PrintPatternInZone benötigt die Liste der zu druckenden Punkte, die Position der Zone, einige Druckparameter wie die Temperatur

  • TakePhotoOfZone benötigt die Zone, in die das Foto aufgenommen werden soll, den Pfad, in dem das Foto gespeichert wird, die Abmessungen usw.

Wenn ich dann anrufe execute(), verwenden die Jobs die spezifischen Informationen, die sie benötigen, um die Aktion zu realisieren, die sie ausführen sollen.

Das einzige Mal, dass ich den konkreten Typ eines Jobs verwenden muss, ist, wenn ich diese Informationen ausfülle oder anzeige (wenn a TakePhotoOfZone Jobausgewählt ist, wird ein Widget angezeigt, das die Parameter für Zone, Pfad und Dimensionen anzeigt und ändert).

Die Jobs werden dann in eine Liste von Jobs eingefügt, die den ersten Job annehmen, ihn ausführen (durch Aufrufen AbstractJob::execute()), die zum nächsten weitergehen, weiter und weiter bis zum Ende der Liste. (Deshalb verwende ich Vererbung).

Um die verschiedenen Arten von Parametern zu speichern, verwende ich a JsonObject:

  • Vorteile: gleiche Struktur für jeden Job, kein dynamic_cast beim Setzen oder Lesen von Parametern

  • Problem: Zeiger (auf Patternoder Zone) können nicht gespeichert werden

Gibt es Ihrer Meinung nach eine bessere Möglichkeit, Daten zu speichern?

Dann , wie würden Sie die konkrete Art der SpeicherungJob , es zu benutzen , wenn ich die spezifischen Parameter dieses Typs ändern müssen? JobManagerhat nur eine Liste von AbstractJob*.

Elf Juni
quelle
5
Ihr ShapeManager scheint zu einer Gottklasse zu werden, da er im Grunde alle Setter-Methoden für alle Arten von Formen enthält.
Emerson Cardoso
Haben Sie über ein "Property Bag" -Design nachgedacht? B. changeValue(int shapeIndex, PropertyKey propkey, double numericalValue)wo PropertyKeykann eine Aufzählung oder eine Zeichenfolge sein, und "Breite" (was bedeutet, dass der Aufruf des Setters den Wert der Breite aktualisiert) gehört zu den zulässigen Werten.
Rwong
Obwohl die Eigenschaftstasche von einigen als OO-Anti-Muster angesehen wird, gibt es Situationen, in denen die Verwendung der Eigenschaftstasche das Design vereinfacht, wobei jede andere Alternative die Dinge komplizierter macht. Um festzustellen, ob Property Bag für Ihren Anwendungsfall geeignet ist, sind jedoch weitere Informationen erforderlich (z. B. wie der GUI-Code mit dem Getter / Setter interagiert).
Rwong
Ich habe das Design der Eigenschaftstasche in Betracht gezogen (obwohl ich den Namen nicht kannte), aber mit einem JSON-Objektcontainer. Es könnte sicher funktionieren, aber ich dachte, es sei kein elegantes Design und es könnte eine bessere Option geben. Warum wird es als OO-Anti-Pattern angesehen?
Elf Juni
Wie kann ich beispielsweise einen Zeiger speichern, um ihn später zu verwenden?
Elf Juni

Antworten:

10

Ich möchte auf Emerson Cardosos "anderen Vorschlag" eingehen, da ich glaube, dass dies im allgemeinen Fall der richtige Ansatz ist - obwohl Sie natürlich andere Lösungen finden können, die für ein bestimmtes Problem besser geeignet sind.

Das Problem

In Ihrem Beispiel verfügt die AbstractShapeKlasse über eine getType()Methode, die im Grunde den konkreten Typ identifiziert. Dies ist im Allgemeinen ein Zeichen dafür, dass Sie keine gute Abstraktion haben. Der springende Punkt beim Abstrahieren ist schließlich, dass man sich nicht um die Details des konkreten Typs kümmern muss.

Falls Sie damit nicht vertraut sind, sollten Sie sich über das Open / Closed-Prinzip informieren. Es wird oft anhand eines Formbeispiels erklärt, damit Sie sich wie zu Hause fühlen.

Nützliche Abstraktionen

Ich nehme an, Sie haben das eingeführt, AbstractShapeweil Sie es für etwas nützlich fanden. Höchstwahrscheinlich muss ein Teil Ihrer Anwendung den Umfang der Formen kennen, unabhängig von der Form.

Dies ist der Ort, an dem Abstraktion Sinn macht. Da sich dieses Modul nicht mit konkreten Formen befasst, kann es AbstractShapenur davon abhängen . Aus dem gleichen Grund wird die getType()Methode nicht benötigt - Sie sollten sie also entfernen.

Andere Teile der Anwendung funktionieren nur mit einer bestimmten Form, z Rectangle. Diese Bereiche profitieren nicht von einer AbstractShapeKlasse, daher sollten Sie sie dort nicht verwenden. Um diesen Teilen nur die richtige Form zu geben, müssen Sie Betonformen separat speichern. (Sie können sie AbstractShapezusätzlich speichern oder im laufenden Betrieb kombinieren).

Minimierung des Betonverbrauchs

Daran führt kein Weg vorbei: An einigen Stellen benötigen Sie die Betontypen - zumindest während des Baus. Manchmal ist es jedoch am besten, die Verwendung von Betontypen auf einige genau definierte Bereiche zu beschränken. Diese getrennten Bereiche haben ausschließlich den Zweck, sich mit den verschiedenen Typen zu befassen - während die gesamte Anwendungslogik aus ihnen herausgehalten wird.

Wie erreichen Sie das? In der Regel durch Einführung weiterer Abstraktionen, die die vorhandenen Abstraktionen widerspiegeln können oder nicht. Zum Beispiel muss Ihre GUI nicht wirklich wissen, um welche Art von Form es sich handelt. Es muss nur bekannt sein, dass es auf dem Bildschirm einen Bereich gibt, in dem der Benutzer eine Form bearbeiten kann.

So können Sie eine Zusammenfassung definieren , ShapeEditViewfür die Sie haben RectangleEditViewund CircleEditViewImplementierungen, die die eigentlichen Textfelder für Breite / Höhe oder Radius halten.

In einem ersten Schritt können Sie ein erstellen, RectangleEditViewwann immer Sie ein erstellen, Rectangleund es dann in ein einfügen std::map<AbstractShape*, AbstractShapeView*>. Wenn Sie die Ansichten lieber nach Bedarf erstellen möchten, können Sie stattdessen Folgendes tun:

std::map<AbstractShape*, std::function<AbstractShapeView*()>> viewFactories;
// ...
auto rect = new Rectangle();
// ...
auto viewFactory = [rect]() { return new RectangleEditView(rect); }
viewFactories[rect] = viewFactory;

In beiden Fällen muss sich der Code außerhalb dieser Erstellungslogik nicht mit konkreten Formen befassen. Als Teil der Zerstörung einer Form müssen Sie natürlich die Fabrik entfernen. Natürlich ist dieses Beispiel zu stark vereinfacht, aber ich hoffe, die Idee ist klar.

Auswahl der richtigen Option

In sehr einfachen Anwendungen stellen Sie möglicherweise fest, dass eine schmutzige (Gieß-) Lösung Ihnen nur das Beste für Ihr Geld gibt.

Das explizite Verwalten separater Listen für jeden Betontyp ist wahrscheinlich der richtige Weg, wenn Ihre Anwendung sich hauptsächlich mit konkreten Formen befasst, aber einige Teile enthält, die universell sind. Hier ist es sinnvoll, nur insoweit zu abstrahieren, als die gemeinsame Funktionalität dies erfordert.

Es lohnt sich im Allgemeinen, den ganzen Weg zu gehen, wenn Sie eine Menge Logik haben, die mit Formen arbeitet, und die genaue Art der Form ist wirklich ein Detail für Ihre Anwendung.

doubleYou
quelle
Ihre Antwort gefällt mir sehr gut, Sie haben das Problem perfekt beschrieben. Das Problem, mit dem ich konfrontiert bin, ist nicht wirklich bei Shapes, sondern bei verschiedenen Jobs für einen 3D-Drucker (z. B. PrintPatternInZoneJob, TakePhotoOfZone usw.) mit AbstractJob als Basisklasse. Die virtuelle Methode ist execute () und nicht getPerimeter (). Die einzige Zeit, in der ich konkrete Verwendung verwenden muss, besteht darin, die spezifischen Informationen, die ein Job benötigt (Liste der Punkte, Position, Temperatur usw.), mit einem bestimmten Widget zu füllen. In diesem speziellen Fall scheint es nicht angebracht zu sein, jedem Job eine Ansicht beizufügen, aber ich sehe nicht ein, wie ich Ihre Vision an meine Arbeit anpassen kann.
Elf Juni
Wenn Sie keine separaten Listen führen möchten, können Sie einen viewSelector anstelle einer viewFactory verwenden : [rect, rectView]() { rectView.bind(rect); return rectView; }. Dies sollte übrigens natürlich im Präsentationsmodul erfolgen, zB in einem RectangleCreatedEventHandler.
doubleYou
3
Versuchen Sie jedoch, dies nicht zu überentwickeln. Der Nutzen der Abstraktion muss immer noch die Kosten der zusätzlichen Installation überwiegen. Manchmal ist eine gut platzierte Besetzung oder eine separate Logik vorzuziehen.
doubleYou
2

Ein Ansatz wäre, Dinge allgemeiner zu gestalten, um zu vermeiden, dass bestimmte Typen gegossen werden .

Sie können in der Basisklasse einen grundlegenden Getter / Setter für Float-Eigenschaften " dimension " implementieren , der einen Wert in einer Map basierend auf einem bestimmten Schlüssel für den Eigenschaftsnamen festlegt. Beispiel unten:

class AbstractShape
{
public :
    typedef enum{
        SQUARE = 0,
        CIRCLE,
    } SHAPE_TYPE;

    AbstractShape(SHAPE_TYPE type):m_type(type){}
    virtual ~AbstractShape();

    virtual float computePerimeter() const = 0;

    void setDimension(const std::string& name, float v){ m_dimensions[name] = v; }
    float getDimension() const{ return m_dimensions[name]; }

    SHAPE_TYPE getType() const{return m_type;}

protected :
    const SHAPE_TYPE  m_type;
    std::map<std::string, float> m_dimensions;
};

Dann müssen Sie in Ihrer Manager-Klasse nur eine Funktion implementieren, wie unten:

void ShapeManager::changeShapeDimension(const int shapeIndex, const std::string& dimension, float value){
   m_shapes[shapeIndex]->setDimension(name, value);
}

Anwendungsbeispiel in der Ansicht:

ShapeManager shapeManager;

shapeManager.addShape(new Circle());
shapeManager.changeShapeDimension(0, "RADIUS", 5.678f);
float circlePerimeter = shapeManager.computeShapePerimeter(0);

shapeManager.addShape(new Square());
shapeManager.changeShapeDimension(1, "WIDTH", 2.345f);
float squarePerimeter = shapeManager.computeShapePerimeter(1);

Ein weiterer Vorschlag:

Da Ihr Manager nur den Setter und die Perimeterberechnung (die auch von Shape verfügbar gemacht werden) verfügbar macht, können Sie einfach eine richtige Ansicht instanziieren, wenn Sie eine bestimmte Formklasse instanziieren. Z.B:

  • Instanziieren Sie ein Square und ein SquareEditView.
  • Übergeben Sie die Square-Instanz an das SquareEditView-Objekt.
  • (optional) Anstatt einen ShapeManager zu haben, können Sie in Ihrer Hauptansicht immer noch eine Liste der Formen führen.
  • In SquareEditView behalten Sie einen Verweis auf ein Quadrat bei. Dies würde das Casting zum Bearbeiten der Objekte überflüssig machen.
Emerson Cardoso
quelle
Ich mag den ersten Vorschlag und habe bereits darüber nachgedacht, aber es ist ziemlich einschränkend, wenn Sie verschiedene Variablen (Float, Zeiger, Arrays) speichern möchten. Für den zweiten Vorschlag: Wenn das Quadrat bereits instanziiert ist (ich habe in der Ansicht darauf geklickt), woher weiß ich, dass es sich um ein Quadrat * -Objekt handelt? Die Liste, in der die Formen gespeichert sind, gibt eine AbstractShape * zurück .
Elf Juni
@ElevenJune - ja, alle Vorschläge haben ihre Nachteile; Zum ersten müssten Sie etwas Komplexeres als eine einfache Karte implementieren, wenn Sie mehr Arten von Eigenschaften wünschen. Der zweite Vorschlag ändert, wie Sie die Formen speichern. Sie speichern die Grundform in der Liste, müssen jedoch gleichzeitig den Verweis der spezifischen Form auf die Ansicht angeben. Vielleicht könnten Sie mehr Details zu Ihrem Szenario bereitstellen, damit wir bewerten können, ob diese Ansätze besser sind, als einfach einen dynamic_cast durchzuführen.
Emerson Cardoso
@ElevenJune - Der springende Punkt beim Anzeigen des Objekts ist, dass Ihre GUI nicht wissen muss, dass es mit einer Klasse vom Typ Square funktioniert. Das Ansichtsobjekt bietet das, was zum "Anzeigen" des Objekts erforderlich ist (was auch immer Sie definieren), und intern weiß es, dass es eine Instanz einer Square-Klasse verwendet. Die GUI interagiert nur mit der SquareView-Instanz. Daher können Sie nicht auf eine 'Quadrat'-Klasse klicken. Sie können nur auf eine SquareView-Klasse klicken. Durch Ändern der Parameter in SquareView wird die zugrunde liegende Square-Klasse aktualisiert.
Dunk
... Mit diesem Ansatz können Sie Ihre ShapeManager-Klasse sehr gut loswerden. Dies wird mit ziemlicher Sicherheit Ihr Design vereinfachen. Ich sage immer, wenn Sie eine Klasse als Manager bezeichnen, nehmen Sie an, dass es sich um ein schlechtes Design handelt, und finden Sie etwas anderes heraus. Manager-Klassen sind aus einer Vielzahl von Gründen schlecht, insbesondere wegen des Problems der Gott-Klasse und der Tatsache, dass niemand weiß, was die Klasse tatsächlich tut, kann und was nicht, weil Manager alles tun können, was auch nur tangential mit dem zusammenhängt, was sie verwalten. Sie können darauf wetten, dass die Entwickler, die Ihnen folgen, das ausnutzen, was zu dem typischen großen Schlammball führt.
Dunk
1
... Sie sind bereits auf dieses Problem gestoßen. Warum um alles in der Welt wäre es für einen Manager sinnvoll, derjenige zu sein, der die Dimensionen einer Form ändert? Warum sollte ein Manager den Umfang einer Form berechnen? Falls Sie es nicht herausgefunden haben, mag ich den "Ein weiterer Vorschlag".
Dunk