Designfrage: Attribut VOR Aufruf checken?



  • Abend,

    kleine Designfrage: Wenn eine Klasse ein Attribut mIsVisible besitzt und gewisse Methoden nur durchlaufen sollen, wenn mIsVisible = true ist, sollte man das Attribut dann VOR Aufruf checkern oder IN der Methode?

    Sprich:
    A) VOR Aufruf checken:

    MyClass* m = ...
    if(m->isVisible())
       m->method();  // MyClass::method() pr├╝ft NICHT mehr den Status von MyClass::mIsvisible ab
    

    oder
    ­čśÄ In der Methode checken:

    MyClass::method() {
       if(!mIsVisible)
          return;
       ...
    }
    
    MyClass* m = ...
    m->method(); // Aufruf unabh├Ąngig vom Status von MyClass::mIsVisible
    

    Was findet ihr besser?



  • kleine Designfrage: Wenn eine Klasse ein Attribut mIsVisible besitzt und gewisse Methoden nur durchlaufen sollen, wenn mIsVisible = true ist, sollte man das Attribut dann VOR Aufruf checkern oder IN der Methode?

    Kommt darauf an. Wenn diese Methoden immer nur dann durchlaufen sollen, wenn mIsVisible == true ist, dann Methode 2, sonst Methode 1.

    Schlie├člich ist es Bl├Âdsinn, wenn man vor jeden Funktionsaufruf von method() eine Abfrage machen muss.

    Edit: Gibt auch F├Ąlle wo man es anders machen sollte. Aber dazu hast du zu wenig Hintergrund/Kontext gegeben.



  • Ich w├╝rde kategorisch immer VOR dem Aufruf checken lassen.
    Wenn man eine Methode aufruft erwartet man auch, dass etwas passiert.

    Ausserdem kann es unn├Âtig Performance kosten. Angenommen hast ein solches Objekt und musst eine Funktion 1 Million mal hintereinander aufrufen immer mit einem anderen Parameter. Dann ist es besser, den Check einmal ausserhalb der Schleife zu machen als 1 Million mal innerhalb.

    Vor Fehlern sch├╝tzen muss man sich nat├╝rlich trotzdem. Daher solltest du in jeder solchen Funktion ein assert auf die Eigenschaft haben assert(isVisible()) .

    Wichtig ist vor allem, dass die Methoden nicht gemischt werden. Sonst muss vor jedem Aufruf ├╝berlegt werden, ob man noch etwas testen muss oder nicht. So passieren dann Fehler. Also immer das gleiche verwenden.

    Die raren Ausnahmen sind delete und die IO-Streams, heute mit Exceptions w├Ąre das sowieso anders gel├Âst worden.



  • B ist besser, weil man die Methode nicht falsch aufrufen kann.
    Eventuell lohnt es sich aber, die Sichtbarkeitseigenschaft aus der Klasse herauszuziehen und anders auszudr├╝cken.

    Die "sichtbaren" Objekte in einer gesonderten Liste f├╝hren:

    struct Renderable
    {
    	void render();
    };
    
    struct Scene
    {
    	void setVisible(Renderable &thing)
    	{
    		m_visibleThings.insert(&thing);
    	}
    
    	void setInvisible(Renderable &thing)
    	{
    		m_visibleThings.erase(&thing);
    	}
    
    	void render()
    	{
    		for (Renderable * const entry : m_visibleThings)
    		{
    			entry->render();
    		}
    	}
    
    private:
    
    	std::set<Renderable *> m_visibleThings;
    };
    

    Decorator pattern:

    struct Renderable
    {
    	virtual ~Renderable();
    	virtual void render() = 0;
    };
    
    struct Hideable : Renderable
    {
    	explicit Hideable(std::unique_ptr<Renderable> realThing, bool isVisible = true)
    		: m_realThing(std::move(realThing))
    		, m_isVisible(isVisible)
    	{
    	}
    
    	void setVisibility(bool isVisible)
    	{
    		m_isVisible = isVisible;
    	}
    
    	virtual void render() override
    	{
    		if (m_isVisible)
    		{
    			m_realThing->render();
    		}
    	}
    
    private:
    
    	std::unique_ptr<Renderable> m_realThing;
    	bool m_isVisible;
    };
    


  • HintergrundKontext schrieb:

    Ich w├╝rde kategorisch immer VOR dem Aufruf checken lassen.
    (...)

    Dann w├╝rdest du kategorisch schwer wartbaren und sehr fehleranf├Ąlligen Code schreiben.
    Solche Dinge checkt man ├╝blicherweise in der Funktion/Methode.
    Und in Spezialf├Ąllen nochmal ausserhalb.
    (z.B. wenn man wirklich viele Objekte hat, von denen die allermeisten invisible sind, kann es Sinn machen den Check in die Schleife rauszuziehen die die Funktion/Methode aufruft)



  • TyRoXx:
    Wobei das ja nur Decorator ist, wenn Renderable dann tats├Ąchlich schon die Render-Funktionalit├Ąt hat. Aber das hast Du jetzt auch angenommen, richtig?

    hustbaer:
    Bei Massenaufruf w├╝rdest Du es zus├Ątzlich von au├čen aufrufen? Das w├╝rde doch nicht helfen, die Pr├╝fung geschieht intern doch sowieso stets noch. Oder meinst Du, Du bietest dann zwei Versionen der Funktion an?

    Ich finde diesen Massenaufruf jedoch eh etwas gek├╝nstelt. In TEs Beispiel kann das so ja nicht geschehen, da die Sichtbarkeit f├╝r jedes Objekt anders sein kann und somit gepr├╝ft werden muss. Der interessante Fall ist doch, wenn man eine Gruppe gebildet hat, in welcher dieses gemeinsame Pr├╝fattribut vorhanden ist. Dann jedoch kann ein einzelnes Objekt dieses Attribut doch ├╝berhaupt nicht kennen und somit kann die Pr├╝fung doch gar nicht intern geschehen.

    Die externe Pr├╝fung sollte sich eben auf Attribute beziehen, die nicht dem Objekt selbst zuzuschreiben und kritisch (im Sinne von: die Funktion darf auf keinen Fall aufgerufen werden, wenn das Attribut z.B. false ist) sind. Wenn es sich um externe Eigenschaften handelt, kann die Pr├╝fung aber sowieso nicht intern geschehen; und dann sollte die Methode aber nat├╝rlich trotzdem funktionieren, was sie aber sowieso tut.

    Mir fehlt hier irgendwie das Anwendungsbeispiel, wo eine externe Pr├╝fung der Internas einen Vorteil bieten w├╝rde, ich sehe bisher keinen.



  • Dein Text ist f├╝r mich irgendwie sehr verwirrend.
    Im Beispiel des OP ist es ganz klar ein "internes" Attribut, ├╝berlegungen ├╝ber "externe" Attribute er├╝brigen sich daher denke ich.

    Eisflamme schrieb:

    hustbaer:
    Bei Massenaufruf w├╝rdest Du es zus├Ątzlich von au├čen aufrufen? Das w├╝rde doch nicht helfen, die Pr├╝fung geschieht intern doch sowieso stets noch. Oder meinst Du, Du bietest dann zwei Versionen der Funktion an?

    Nein, ich w├╝rde nur eine Version der Funktion anbieten.
    Wenn gerendert wird, dann tut der Test eines bools nicht wirklich weh, weil das Rendern sowieso viel l├Ąnger dauert.

    Bleibt also noch ein Fall den man optimieren kann, und zwar dass man viele viele Objekte hat von denen nur ganz wenige gerendert werden.
    Und genau da macht ein zus├Ątzlicher, "externer" Test u.U. Sinn. Und zwar dann, wenn die Render-Funktion nicht inline erweitert wird, der Test aber schon.
    Dann spart man sich n├Ąmlich die vielen Funktionsaufrufe f├╝r die vielen Objekte die eh nicht gerendert werden sollen.

    Ich finde diesen Massenaufruf jedoch eh etwas gek├╝nstelt. In TEs Beispiel kann das so ja nicht geschehen, da die Sichtbarkeit f├╝r jedes Objekt anders sein kann und somit gepr├╝ft werden muss.

    Ja, darauf beziehe ich mich ja auch.
    Ich rede ja nicht von if - for - render sondern von for - if - render .

    Der interessante Fall ist doch, wenn man eine Gruppe gebildet hat, in welcher dieses gemeinsame Pr├╝fattribut vorhanden ist. Dann jedoch kann ein einzelnes Objekt dieses Attribut doch ├╝berhaupt nicht kennen und somit kann die Pr├╝fung doch gar nicht intern geschehen.

    Wieso sollte das der interessante Fall sein? Normalerweise besitzt das Objekt sein eigenes "visible" Attribut, den von dir beschriebenen Fall kann es dann gar nicht geben.

    Mir fehlt hier irgendwie das Anwendungsbeispiel, wo eine externe Pr├╝fung der Internas einen Vorteil bieten w├╝rde, ich sehe bisher keinen.

    Siehe oben. Ich meine auch nicht dass man das ├Âfter so machen sollte. Ich wollte damit nur sagen: es kann F├Ąlle geben wo das Sinn macht - wenn auch nicht viele.



  • RobbenCaptn schrieb:

    kleine Designfrage: Wenn eine Klasse ein Attribut mIsVisible besitzt und gewisse Methoden nur durchlaufen sollen, wenn mIsVisible = true ist, sollte man das Attribut dann VOR Aufruf checkern oder IN der Methode?

    Gegenfrage: Stellt es einen Programmier-/Logikfehler dar, wenn die Methode aufgerufen wird, obwohl isVisible gleich false ist? Dann w├╝rde ich eher auf ein assert zur├╝ckgreifen.

    Mal ein ganz gruseliges Beispiel: Ein Programmierer schreibt eine Funktion, die einen g├╝ltigen Pointer als Argument erwartet. Etwas anderes als ein g├╝ltiger Pointer stellte in diesem Fall einen massiven Programmfehler dar.

    Nun f├╝gte dieser Programmierer (wahrscheinlich durch einen Absturz o.├Ą. motiviert) am Anfang dieser Funktion ein "if (ptr == NULL) return;" (oder ├Ąquivalent) hinzu. Klar, der Absturz war vielleicht weg. Damit aber auch jede Chance den Fehler in einem anderen Teil des Quellcodes ├╝berhaupt erst zu entdecken. Wird ein NULL-Pointer ├╝bergeben (dessen reine Existenz an dieser Stelle auf einen Fehler in einem anderen Code-Bereich hinweist), kehrte die Funktion ohne jedes Mucken (aber auch ohne irgendeine Arbeit zu verrichten) einfach zur├╝ck. Du kannst dir vorstellen, dass das Finden von Fehlern in derart geschriebenem Code keine sch├Âne Angelegenheit ist.

    Mit defensiver Programmierung hat das auch ├╝berhaupt nichts zu tun. Ein klar illegaler Parameter sollte niemals dazu f├╝hren, dass die Funktion still und leise einfach gar nichts macht. So versteckt man nur Bugs.

    Sowas willst du aber nicht machen, oder?


Log in to reply