Exception oder nullptr?

[sboc]

Hallo,

ich bin aktuell über einige Exception-Artikel gestolpert und nun völlig unsicher, wann ich eine Exception werfen soll oder wann ein nullptr als Rückgabewert im Fehlerfall genügt.

Angenommen es gibt eine Klasse, die Ressourcen in einem Array verwaltet. Das Array wird zu Beginn in festgelegter Größe erzeugt und mit "Leerdaten" gefüllt. Weiterhin besitzt diese Klasse die zu verwaltenden Objekte.

Nun fallen mir mindestens 3 Operationen ein, bei denen ich mir teilweise unsicher bin, was ich im Fehlerfall tun sollte. Möglichkeit 1 wäre komplett über Exceptions zu gehen:

void set( std::size_t index, const Foo& value ); // 1
const Foo& get( std::size_t index ) const; // 2
void create( std::size_t index ); // 3

(1) setzt einen neuen Wert. Darf aber nur gehen, wenn der index gültig ist.
(2) holt einen Wert. Darf aber nur gehen, wenn index gültig ist und bei index ein Wert liegt (d.h. vorher per create() angelegt bzw. set() gesetzt).
(3) erzeugt einen Wert. Darf aber nur gehen, wenn index gültig ist und bei index noch kein Wert liegt.

Möglichkeit 2 wären bool'sche Rückgabewerte und nullptr:

bool set( std::size_t index, const Foo& value ); // 1
const Foo* get( std::size_t index ) const; // 2
bool create( std::size_t index ); // 3

Bei der 2. Möglichkeit könnte ich mir auch eine Mischung vorstellen. Das heißt: Exception werfen, falls index ungültig ist, denn in diesem Fall liegt ein Programmierfehler vor. Hingegen ist für so etwas ja assert() da...

Es wäre super, wenn jemand dazu ein paar Ideen/Erfahrungen hätte. Ich möchte meinen Code gerne so schreiben, dass Fehler schnell auffallen und es ergo schwer ist, ihn falsch zu benutzen. Aber anders herum erscheint mit manches in Möglichkeit 1 so umständlich (z.B. müsste ein Benutzer vorher abfragen, ob ein Wert existiert, bevor er ihn mit get() holen kann, damit der Fehlerfall vermieden wird und es nicht zu einer Exception kommt).

[cooky451]

bool set( std::size_t index, const Foo& value, bool throw_exception = false );
const Foo* get( std::size_t index, bool throw_exception = false ) const;
bool create( std::size_t index, bool throw_exception = false );

Wenn die Klasse eh nur in einem größeren Kontext Sinn macht, in dem Exceptions auf höherer Ebene gefangen werden (z.B. in einem Spiel), dann kannst du auch gleich Variante 1 nehmen. Ansonsten weißt du nicht wie der "Nutzer" der Klasse diese Funktionen nutzen möchte, warum also nicht ihm die Wahl überlassen?

[Smilies in diesem Beitrag]

sboc schrieb:

Hingegen ist für so etwas ja assert() da...

Genau, und das eigentlich für alle 3.

Es wäre super, wenn jemand dazu ein paar Ideen/Erfahrungen hätte. Ich möchte meinen Code gerne so schreiben, dass Fehler schnell auffallen und es ergo schwer ist, ihn falsch zu benutzen. Aber anders herum erscheint mit manches in Möglichkeit 1 so umständlich (z.B. müsste ein Benutzer vorher abfragen, ob ein Wert existiert, bevor er ihn mit get() holen kann, damit der Fehlerfall vermieden wird und es nicht zu einer Exception kommt).

Du baust irgendwie sowas wie std::vector nach, aber create mit index könnte auch wieder std::map sein. Sieht irgendwie komisch aus dein Design. Warum spricht man Ressourcen per index an? Was für Ressourcen überhaupt?

cooky451 schrieb:

bool set( std::size_t index, const Foo& value, bool throw_exception = false );
const Foo* get( std::size_t index, bool throw_exception = false ) const;
bool create( std::size_t index, bool throw_exception = false );

Schrecklich.

[mgaeckler]

Hallo,

der Vorteil von Exceptions ist, daß im Aufrufstack zwischen der Fehlerursache und der Fehlerbehandlung viele Routinen stehen können, die sich nicht um den Fehler kümmern müssen, weil der erforderliche Code vom Compiler automatisch generiert wird. Nur solche Funktionen, die Ressourcen angefordert haben, müssen auf den Fehler reagieren, brauchen sich aber nicht um die Art des Fehler kümmern:

void *foo( void )
{
    void *ptr = malloc( 100000 );
    try
    (
         do_something( ptr );
    }
    catch( ... )
    {
        free( ptr );
        throw;
    }
    return ptr;
}

Funktionen, die statt eine Exception zu werfen einen Fehlercode zurückliefern, müssen sich darauf verlassen, daß der direkte Aufrufer dann auch darauf reagiert.

Wenn dies der Fall ist, benutze ich lieber den Fehlercode, weil das für mich meistens einfacher zu realisieren ist:

FILE *foo( void )
{
    FILE *ptr = fopen( "temp.txt", "r" );
    if( !ptr )
    {
         ptr = fopen( "temp.old", "r" );
    }

    return ptr;
}

Wenn fopen stattdessen 'ne Exception werfen würde, müsste der Code so ausschauen:

FILE *foo( void )
{
    FILE *ptr;

    try
    {
        ptr = fopen( "temp.txt", "r" );
    }
    catch( ... )
    {
         ptr = fopen( "temp.old", "r" );
    }

    return ptr;
}

Ist jetzt vielleicht ein blödes Beispiel, weil mir auf die schnelle nix besseres einfällt, aber ich denke man sieht, was ich meine.

Es kommt halt auch immer darauf an, wie das Programm im Fehlerfall reagiert.

Wenn Du ganz ohne Exceptions auskommst, kann der Compiler auch den ganzen automatisch generierten Code weglassen, was Dein Programm unter Umständen schneller machen kann. Ich denke aber, daß in vielen Fällen der Vorteil der Exceptions aber überwiegt.

mfg Martin

[cooky451]

Smilies in diesem Beitrag schrieb:

Schrecklich.

Weil?

[Smilies in diesem Beitrag]

cooky451 schrieb:

Smilies in diesem Beitrag schrieb:

Schrecklich.

Weil?

Weil ich schrecklichen Code erkenne. :p
Was bringt das, wenn ich es sowieso nur da setzen kann, wo ich auch gleich den Rückgabewert prüfen kann? Da kommen mir nur so komische Fragen wie: Muss man den return Wert jetzt immer noch anschauen? Haben die das gebaut, weil der Code über irgendeine Schnittstelle aufgerufen wird, die keine Exceptions kann?

[cooky451]

Smilies in diesem Beitrag schrieb:

Was bringt das, wenn ich es sowieso nur da setzen kann, wo ich auch gleich den Rückgabewert prüfen kann?

Na ja, dass man sich beide Möglichkeiten offen hält? Man könnte auch gleich ein obj.enableExceptions(); oder so einführen, aber grundsätzlich ist die Idee ja die Gleiche. Denn niemand will solchen Code sehen:

int main()
{
  KomischeKlasse obj;
  try
  {
    obj.foo();
  }
  catch (KomischeKlasseException &e)
  {
  }
}

Und so etwas ist natürlich noch schlimmer:

int main()
{
  KomischeKlasse obj;
  if (obj.foo())
  {
    if (obj.bar())
    {
      if(obj.baz())
      {
      }
      else
      {
      }
    }
    else
    {
    }
  }
  else
  {
  }
}

Solange man also beim schreiben der Klasse nicht erraten kann, wie das Programm des Nutzers aussieht, sollte man ihm doch ruhig die Wahl lassen. assert() würde ich dagegen nur in Debug-Code nutzen und auch nur um auf offensichtliche Programmierfehler aufmerksam zu machen. Um Laufzeitfehler (im Sinne von "Datei nicht vorhanden" etc.) abzufangen halte ich assert() doch für mehr als unpassend, den Fehler kann man dann ja auch nicht mal mehr behandeln.

[314159265358979]

cooky451 schrieb:

int main()
{
  KomischeKlasse obj;
  try
  {
    obj.foo();
  }
  catch (KomischeKlasseException &e)
  {
  }
}

Äh, nein. Genau so solls sein.

[cooky451]

314159265358979 schrieb:

Äh, nein. Genau so solls sein.

Das ist dir lieber als:

int main()
{
  KomischeKlasse obj;
  if (obj.foo())
  {
  }
}

? Komisch, ich habe dich noch nie mit Filestream-Exceptions hantieren sehen.

[314159265358979]

Bastelst du also bei jeder deiner Klassen dieses setstate-Zeugs drumherum?

[cooky451]

314159265358979 schrieb:

Bastelst du also bei jeder deiner Klassen dieses setstate-Zeugs drumherum?

Solange ich nicht einschätzen kann wie die Klasse genutzt wird, mache ich es wie oben vorgeschlagen. Es mag sicher Klassen geben bei denen es sinnvoll ist immer zu werfen, aber das ist hier ja nicht der Fall.

[destroying angel]

So ein Flag bei allen Funktionen verschandelt die ganze API! Wenn jemand lieber mit exceptions arbeitet, muss er immer ein "true" mitgeben. Wenn der letzte ordentliche Parameter ein bool ist, wirds auch noch ganz verzwickt, wenn man das letzte "true" für das "throwException-Flag" hält. Braucht man dafür ein false und geht später über den Code, ist die Versuchung groß, den Code anzupassen, und das false in ein true zu ändern, obwohl es gar nix mehr mit dem exception-flag zu tun hat.
Außerdem muss jede Funktion das Flag prüfen und entsprechend den einen oder anderen Codepfad ausführen - das bläht unnötig auf.

Da jemand entweder volles Rohr auf Exceptions steht (wo sie auch Sinn machen!) oder sie abgrundtief hasst (mir ist noch kaum jemand dazwischen begegnet), ist die einzige saubere Lösung, wenn man wirklich beides anbieten will, ein Präprozesor-Flag, das exceptions anmacht oder auf returncodes ausweicht.

[Shade Of Mine]

cooky451 schrieb:

314159265358979 schrieb:

Bastelst du also bei jeder deiner Klassen dieses setstate-Zeugs drumherum?

Solange ich nicht einschätzen kann wie die Klasse genutzt wird, mache ich es wie oben vorgeschlagen. Es mag sicher Klassen geben bei denen es sinnvoll ist immer zu werfen, aber das ist hier ja nicht der Fall.

Schlechte Idee.
Ein sinnvoller Stil ist immer besser als alle Stile die man findet zu vermischen.

Entweder man wirft Exception oder man wirft keine. Umschalten ist dämlich. Gibt auch genug Diskussionen warum es in der C++ Library mit den Filestreams ziemlich kacke ist.

Das Flag ist halt furchtbar, weil man nie weiss was jetzt passiert. Ist der Code Exception sicher oder nicht? Man weiss es nicht weil das Ding vielleicht wirft, vielleicht auch nicht. Also muss ich immer alles checken. Das ist einfach nicht gut.

[cooky451]

destroying angel schrieb:

..obwohl es gar nix mehr mit dem exception-flag zu tun hat.

Ja, das könnte passieren. Aber nicht bei drei Funktionen mit 1-2 Parametern. Eventuell müsste man dann doch noch auf eine flag-Funktion ausweichen.

destroying angel schrieb:

Außerdem muss jede Funktion das Flag prüfen und entsprechend den einen oder anderen Codepfad ausführen - das bläht unnötig auf.

Nein. Die Bedingung die die Exception auslösen soll muss eh geprüft werden, da wird halt noch ein " && throw_exception" angehangen. Was den Code aufbläht, sind #ifdefs in jeder Funktion.

Shade Of Mine schrieb:

Ein sinnvoller Stil ist immer besser als alle Stile die man findet zu vermischen.

Aber nicht wenn man den Stil mit dem die Klasse genutzt werden soll gar nicht kennt.

Shade Of Mine schrieb:

Das Flag ist halt furchtbar, weil man nie weiss was jetzt passiert.

Kannst du mir dafür mal ein Beispiel nennen?

[dot]

sboc schrieb:

void set( std::size_t index, const Foo& value ); // 1
const Foo& get( std::size_t index ) const; // 2
void create( std::size_t index ); // 3

Es hängt natürlich vom Anwendungsfall ab, aber ich würd im Allgemeinen da eher in allen 3 Fällen wenn überhaupt dann ein assert() verwenden. Alternativ könnte man dem Container-Template eine Range-Checking-Policy als Argument geben, die dann wahlweise eine Exception wirft. Oder man macht es so wie die std:: Container und bietet zwei Methoden an, eine die eine Exception wirft und eine die's nicht tut.
Die Methode create() könnte den alten Wert zurückgeben, oder einen bool der sagt ob der Wert schon vorhanden war und überschrieben wurde.
Rückgabewerte für Index of out Bounds wären da imo jedenfalls ziemlich fehl am Platz.

[Smilies in diesem Beitrag]

cooky451: Glaub es einfach, mindestens 3 Leute sagen schon, dass es schrecklich ist.

sboc: Warum brauchst man überhaupt eine create und eine set Methode? Die create Methode hat nicht mal Zusatzparameter, wie etwas erstellt werden soll. Das sieht alles sehr komisch aus. Was soll das am Ende tun (Beispiel).

[sboc]

Zunächst vielen Dank für die Antworten. Da habe ich ja fast vermutet, dass ein kleiner Glaubenskrieg ausbricht. :xmas1:

Smilies in diesem Beitrag schrieb:

Warum brauchst man überhaupt eine create und eine set Methode?

Das ist nur wg. des Beispiels so. Diese drei Operationen sind mir prompt eingefallen, bei denen ich mir unsicher bin, wie ich das handeln soll. Tatsächlich würden z.B. nur "set" und "get" gemeinsam in einer Klasse auftauchen, "create" dann separat in einer anderen.

dot schrieb:

Es hängt natürlich vom Anwendungsfall ab, aber ich würd im Allgemeinen da eher in allen 3 Fällen wenn überhaupt dann ein assert() verwenden.

Das würde mir (technisch) auch völlig ausreichen. Blöderweise verträgt sich das assert() mit Boost.Test, das ich einsetze, nicht so gut. Auf Exceptions testen ist einfacher.

Im Falle der "get"-Methode tendiere ich mittlerweile zu einer Exception/nullptr-"Kombination". Exception werfen, wenn der Parameter unsinnig ist, d.h. wirklich ein Programmierfehler vorliegt und nullptr (bzw. Fehlerwert) liefern, wenn zwar der Parameter gültig ist, an geeigneter Stelle aber nichts gefunden wird.

Ich möchte euch aber gerne noch mit einem konkreteren Beispiel verwöhnen, der vorige Code scheint teilweise mehr verwirrt als geholfen zu haben:

class Map {
	public:
		static const unsigned int INVALID_ID = std::numeric_limits<unsigned int>::max();

		Map( unsigned int size ) :
			m_tiles( size * size, INVALID_ID ),
			m_size( size )
		{
		}

		void set( unsigned int x, unsigned int y, unsigned int id ) {
			if( x >= m_size || y >= m_size ) {
				throw std::invalid_argument( "x/y doof." );
			}
			else if( id == INVALID_ID ) {
				throw std::invalid_argument( "id doof." );
			}

			m_tiles[y * m_size + x] = id;
		}

		void reset( unsigned int x, unsigned int y ) {
			if( x >= m_size || y >= m_size ) {
				throw std::invalid_argument( "x/y doof." );
			}

			m_tiles[y * m_size + x] = INVALID_ID;
		}

		unsigned int get( unsigned int x, unsigned int y ) const {
			if( x >= m_size || y >= m_size ) {
				throw std::invalid_argument( "x/y doof." );
			}

			return m_tiles[y * m_size + x];
		}

	private:
		std::vector<unsigned int> m_tiles;
		unsigned int m_size;
};

std::vector<unsigned int> ist der Einfachheit halber gewählt, es gibt den Fall auch mit größeren Ressourcen, die dann über Pointer rausgehen.
Prinzipiell habe ich im Beispiel nun statt assert()s Exceptions genutzt und mich im get-Fall entschieden, einen Fehlerwert zurück zu geben, falls zwar die Koordinate korrekt ist, dort aber kein wirklicher Inhalt zu finden ist (INVALID_ID). Und falls wer meckert, dass das ja fast ein purer vector-Wrapper ist: Es gibt selbstverfreilich weitere Operationen.

Was haltet ihr davon?

[Thorgrim]

Ich würde bei diesen Settern nur in der Debugversion mit asserts den Bereich und die IDs prüfen.

In der Releaseversion haben derlei Überprüfungen imho nichts zu suchen.

[dot]

Dann bau dir ein assert() dass eine Exception wirft?
Wenn dein Programmdesign vom Testsystem bestimmt wird, dann ist das Testsystem Mist oder du verwendest es falsch, wenn du mich fragst.

[Smilies in diesem Beitrag]

sboc schrieb:

Blöderweise verträgt sich das assert() mit Boost.Test, das ich einsetze, nicht so gut. Auf Exceptions testen ist einfacher.

Du brauchst auch nicht testen, was passiert, wenn man falsch programmiert.

[sboc]

dot schrieb:

Dann bau dir ein assert() dass eine Exception wirft?
Wenn dein Programmdesign vom Testsystem bestimmt wird, dann ist das Testsystem Mist oder du verwendest es falsch, wenn du mich fragst.

Könnte es nicht auch ein Hinweis darauf sein, dass das Design kein anständiges Testen zulässt? Ich schreibe zuerst Unittests, bevor ich den Produktiv-Code schreibe, und dort möchte ich halt auch Fehler provozieren, wo sie auftauchen sollen.

Du brauchst auch nicht testen, was passiert, wenn man falsch programmiert.

Ich denke schon, dass das sinnvoll ist. Ich möchte ja z.B. durch einen Testfall sicherstellen, dass eine Funktion bei falschen Werten fehlerhaft reagiert. Das wäre vergleichbar damit, als würde man nur testen, ob 15 / 5 = 3, aber nicht ob 15 / 0 = Fehler.

Dass etwas bei anständigen Werten funktioniert, ist ja nur die halbe Miete. Oder aber ich mache es tatsächlich falsch, bin noch recht frisch beim test-driven development.