3. Große Klasse, Redesign

Große Klasse, Redesign

  • ?

    Hallo!

    Eine Klasse 'Image' besitzt Bilddaten und Methoden zur Manipulation dieser
    Bilddaten. operator+=(Image& otherPic) ist die Hauptmethode, die otherPic
    und *this zuerst vorbereitet und dann addiert. Soweit ist das Design der
    Klasse sauber:

    class Image
{
public:
	void operator+=(Image& otherPic);

private:
	unsigned xResolution;
	unsigned yResolution;
	vector<Pixel> vPixels;

	void redEyes();
	void enhanceContrast();
	void denoise();
	void scale();
	void clip();	
	void overlay(vector<Pixels>& vPixels);
};

bla..

Image::operator+=(Image& otherPic)
{
	this->scale();
	this->clip();

	otherPic.scale();
	otherPic.clip()
	otherPic.redEyes();
	otherPic.enhanceContrast();
	otherPic.denoise();
	this->overlay(otherPic.vPixels);	
}

    Das Problem: Bei diesem Design steht beinahe der ganze Code der Software in
    der (bereits riesigen) Klasse 'Image', und das wirkt so, als würde man nicht
    objektorientiert programmieren, sondern hätte globale Variablen, auf die jede
    beliebige Funktion zugreifen kann. -> Unübersichtlich, schwer zu debuggen. Da
    noch viele weitere Funktionen zur Manipulation der Daten zu implementieren
    sind, ist ein anderes Design erforderlich.

    Ich bin mit keiner meiner Ideen zum Redesign so recht glücklich, daher würde
    ich mir gerne Eure Vorschläge ansehen, wie man die Klasse aufteilen oder die
    Funktionen zur Datenmanipulation auslagern kann.

    (Anm: Es geht tatsächlich nicht um Bilddaten, sondern um 3D-Objekte, wo noch
    mehr private Daten in der Klasse sind und manipuliert werden, aber als
    Metapher für die Diskussion ist die obige Image-Klasse besser geeignet.)

    Danke, lg

    0
  • O

    Ich würde die Bildbearbeitungsfunktionen von der Image-Klasse trennen. Ganz grob skizziert vielleicht:

    class Image {
    unsigned xResolution;
    unsigned yResolution;
    vector<Pixel> pixels;
public:
    // Konstruktor, getter, setter, ...
};

class ImageRenderer {
public:
    void render(const Image& image) {
        // ...
    }
};

class ImageFilter {
public:
    virtual void apply(Image& img) = 0;
    virtual ~ImageFilter() {}
};

class RedEyeFilter {
public:
    virtual void apply(Image& img) {
        // Hier rote Augen entfernen
    }
};

class ImageTransformer {
public:
    virtual Image transform(const Image& imgA, const Image& imgB) = 0;
    virtual ~ImageTransformer() {}
};

class Overlayer : ImageTransformer {
public:
    virtual Image transform(const Image& imgA, const Image& imgB) {
        // Bilder übereinander legen und neues Bild zurückgeben
    }
};

    Ja, so in der Art würd ichs jetzt spontan machen.

    0
  • D

    Interessant, dass das Thema in letzer Zeit des öfteren nachgefragt wird.

    Z.B gleich hier:
    http://www.c-plusplus.net/forum/viewtopic-var-t-is-273691.html

    oder hier:
    http://www.c-plusplus.net/forum/viewtopic-var-t-is-273619-and-highlight-is-.html

    Die Threads laufen mehr oder weniger auf das, was OhneName auch sagt heraus.

    0
  • B

    Das was du dort gemacht hat nennt sich God-class oder God-object http://de.wikipedia.org/wiki/God_object und wie schon hier erklärt hilft da nur feiner zu granulieren.

    Gruß Blue-Tec

    0
  • N

    Die Operatorüberladung

    void operator+=(Image& otherPic);

    ist auch äusserst fragwürdig. Am besten fährst du, wenn du einige Konventionen beachtest:

    • Benutze Operatoren nur, wenn sie intuitiv sind. Ich würde hier eine benannte Funktion vorziehen, ein Plus kann in dem Zusammenhang vieles bedeuten.
    • Beachte Const-Correctness. Ein += verändert den rechten Operand nie. Dass du es doch tust, ist ein umso grösseres Zeichen dafür, dass eine benannte Methode besser geeignet wäre.
    • Gebe etwas zurück. Die übliche Konvention bei operator+= ist, eine Referenz auf this zurückzugeben, um Verkettungen zu ermöglichen.
    • Wenn du a += b; schreiben kannst, muss der Ausdruck a = a + b; ebenfalls gültig und semantisch äquivalent mit dem ersten sein.

    Ich würde hier auf Operatorüberladung verzichten. Siehe auch Überladung von Operatoren in C++.

    0
  • A

    Schau dir an wie VTK das macht (oder nimm das gleich, denn es hat sicher 90% der Funktionalität die du brauchst).
    (Bezugsdiagramm hier: http://www.vtk.org/doc/nightly/html/classvtkAlgorithm.html)

    Alles sauber von einer Basisklasse abgeleitet (vtkObject, bzw. hier eher vtkAlgorithm). Das Bild zeigt natürlich nur die erste Ebene der Ableitungen.

    0
  • A

    antialias schrieb:

    Alles sauber von einer Basisklasse abgeleitet...

    "Sauber" ist immer relativ. Ableitungen mögen manche Probleme beheben, bergen aber auch sehr viel Neue. Nichts gegen flache Vererbungshirachien, die hier scheint aber eine tiefe Hierarchie zu sein.

    Gerade nichts aussagende Klassennamen wie TObject, CObject, vtkObject... halte ich häufig für ein Anzeichen schlechtes Designs.

    0
  • ?

    antialias schrieb:

    Schau dir an wie VTK das macht (oder nimm das gleich, denn es hat sicher 90% der Funktionalität die du brauchst).
    (Bezugsdiagramm hier: http://www.vtk.org/doc/nightly/html/classvtkAlgorithm.html)

    Alles sauber von einer Basisklasse abgeleitet (vtkObject, bzw. hier eher vtkAlgorithm). Das Bild zeigt natürlich nur die erste Ebene der Ableitungen.

    Das sieht aus wie etwas, das ich brauchen kann. So wie ich das verstehe,
    werden hier Unix-Pipes nachgebildet. Die Klasse vtkAlgorithm ist ein
    allgemeines Rohr. Die Algorithmen, die von vtkAlgorithm ableiten, werden
    dadurch ebenfalls Rohre mit kompatiblen Anschlußstücken. Diese Anschluß-
    stücke heißen inputPortInformation und outputPortInformation, womit die
    Ein- und Ausgangsdaten der Algorithmen gemeint sind.

    Leider ist das Ding RIESIG, und ich finde mich in der Doku kaum zurecht.
    So weiß ich noch nicht, wo die Abfolge festgelegt wird, d.h. wo die Rohre
    zusammengeschaltet werden. Ein kleineres Projekt nach diesem Paradigma wäre
    günstig. Aber ich lese mich weiter ein. Danke für den Tipp, die Idee macht
    mir Fieber in den Fingern.

    lg

    0
Anmelden zum Antworten