Eigene Fehlerklasse

Hallo zusammen,

ich habe mir eine eigene Fehlerklasse erzeugt und wollte mal fragen ob das aus C++ Sicht so in Ordnung ist oder ob ich da noch ein Problem habe, z.B. das die Fehlermeldungen unter public stehen (oder kann man das so lassen), oder was auch immer ? Funktionieren tut es jedenfalls.

*.h Datei

//---------------------------------------------------------------------------
#ifndef ErrorClassH
#define ErrorClassH

const int MAXMSG = 10;

class MyException
{
 private:
 public:
	AnsiString sErrorMessage[MAXMSG];
	__fastcall MyException();
	virtual AnsiString __fastcall showError() = 0;
};

class InterpreterError : public MyException
{
 private:
	int nr;
 public:
	__fastcall InterpreterError(int a);
	AnsiString __fastcall showError();
};

class UndefinedError : public MyException
{
 private:
 public:
	__fastcall UndefinedError();
	AnsiString __fastcall showError();
};
//---------------------------------------------------------------------------
#endif

*.cpp Datei

//---------------------------------------------------------------------------
#include <vcl.h>
#include <iostream>
#pragma hdrstop

#include "ErrorClass.h"
//---------------------------------------------------------------------------
#pragma package(smart_init)
using namespace std;

__fastcall MyException::MyException()
{
 sErrorMessage[0] = "Error bla bla";
 sErrorMessage[1] = "Error bla bla bla";
 // ...
 // etc.
}
//---------------------------------------------------------------------------

__fastcall InterpreterError::InterpreterError( int a )
{
 nr = a;
 if ( nr >= MAXMSG ) nr = 0;
}

AnsiString __fastcall InterpreterError::showError()
{
 return sErrorMessage[nr];
}
//---------------------------------------------------------------------------

__fastcall UndefinedError::UndefinedError()
{
}

AnsiString __fastcall UndefinedError::showError()
{
 return "Undefined Error";
}
//---------------------------------------------------------------------------

Viele Dank schon mal für eure Tips.

Hallo einzelner,

sehr strange die Klasse.

asc

Trial schrieb:

ich habe mir eine eigene Fehlerklasse erzeugt und wollte mal fragen ob das aus C++ Sicht so in Ordnung ist...

Im ersten Moment sollte dir erstmal bewusst sein das du hier kein AnsiC++ verwendest (Das VCL/C++ Builder-Forum wäre die bessere Wahl gewesen).

Aufgrund eigener Erfahrungen, würde ich bestimmte Notationen vom C++ Builder auch bei eigenen Klassen vermeiden, sofern diese nicht von einer VCL-Klasse abgeleitet sind. So beispielsweise __fastcall, weil dies nicht mit den C++ Sprachmitteln wirklich gut läuft (Zumindestens hatte ich Probleme mit umgedrehten Konstruktorreihenfolgen, inkompatibilitäten zu std::tr1::function etc.).

In der VCL-Hierachie sieht das ganze wieder anders aus.

Trial schrieb:

oder ob ich da noch ein Problem habe, z.B. das die Fehlermeldungen unter public stehen (oder kann man das so lassen), oder was auch immer ? Funktionieren tut es jedenfalls.

Grundsätzlich würde ich public-Eigenschaften meiden. Zumindestens wenn du später noch Änderungen vornehmen willst, sind Getter/Setter etc. eine bessere Wahl.

Zudem würde ich grundsätzlich Eigenschaften dort ansiedeln, wo sie verwendet werden, und nicht zwangsweise in eine Basisklasse verschieben, nur weil man sich dort Schreibaufwand spart. UndefinedError verwendet beispielsweise kein sErrorMessage (Meine negative Meinung zu der ungarischen Notation lasse ich hier mal beiseite).

Davon abgesehen ist sErrorMessage wohl nur für die Hierachie gedacht, daher ohnehin als public-Member falsch (und wenn würde ich hier nur das Lesen ermöglichen).

Zudem würde ich (aber nur als Ergänzung gesagt) statt einem C-Array eher std::tr1::array/boost::array verwenden, da dies bei dir zudem noch Klassen nicht Objektbezogen ist: statisch. Vielleicht wäre auch eine std::map noch besser aufgehoben, zumindestens wenn die Fehlernummern "Lücken" haben können.

Und zudem noch weitere Anmerkungen:
- Stilistisch -
1. Sinnvolle Einrückungsebenen verwenden (1 Zeichen halte ich arg wenig)
2. Jede Anweisung in eine eigene Zeile schreiben, und die Einrückungen beachten.
1&2 erhöhen die Lesbarkeit (s.u.)
3. Du solltest dir mal Initialisierungslisten von Konstruktoren anschauen (Innerhalb des Konstruktorrumpfes werden nur Zuweisungen, nicht Initialisierungen gemacht, die Initialisierung erfolgt im Konstruktorrumpf. In deinen aktuellen Fall noch relativ egal, kann dies später mal relevant sein [Direkte Konstruktoraufrufe, Konstanteninitialisierungen, Referenzzuweisungen...]).

__fastcall InterpreterError::InterpreterError(int a)
:   nr(a) // Initialisierungsliste
{
    if ( nr >= MAXMSG ) // Erste Einrückungsebene
        nr = 0;         // Gehört zum if-Block, daher weitere Einrückung
}

cu André

audacia

Trial schrieb:

ich habe mir eine eigene Fehlerklasse erzeugt und wollte mal fragen ob das aus C++ Sicht so in Ordnung ist

Exception-Klassen sollten mindestens von std::exception ableiten. Wenn du dir das manuelle Verwalten des Speichers für eine bereits im Konstruktor zusammengesetzte Fehlermeldung sparen willst, kannst du auch z.B. von std::runtime_error ableiten.

Das Visualisieren der Fehlermeldung ist außerdem nicht Aufgabe einer Exception-Klasse, sondern der Umgebung, die sie benutzt; showError() hat in MyException eigentlich nichts zu suchen.

Zu allem anderen hat asc sich schon ausführlich geäußert.

Hallo,

erstmal vielen Dank für Eure Tips, die helfen mir prima weiter. Ein paar Fragen habe ich noch:

asc schrieb:

std::tr1::function

Was bedeutet hier tr1 ?

asc schrieb:

Vielleicht wäre auch eine std::map noch besser aufgehoben, zumindestens wenn die Fehlernummern "Lücken" haben können.

Und dann die std::exception zu nehmen oder wie meinst Du das ?

asc schrieb:

3. Du solltest dir mal Initialisierungslisten von Konstruktoren anschauen (Innerhalb des Konstruktorrumpfes werden nur Zuweisungen, nicht Initialisierungen gemacht, die Initialisierung erfolgt im Konstruktorrumpf. In deinen aktuellen Fall noch relativ egal, kann dies später mal relevant sein [Direkte Konstruktoraufrufe, Konstanteninitialisierungen, Referenzzuweisungen...]).
__fastcall InterpreterError::InterpreterError(int a)
:   nr(a) // Initialisierungsliste
{
    if ( nr >= MAXMSG ) // Erste Einrückungsebene
        nr = 0;         // Gehört zum if-Block, daher weitere Einrückung
}

Das war ein guter Tip , war mir nicht bekannt.

audacia schrieb:

Exception-Klassen sollten mindestens von std::exception ableiten.

Kannst Du mir das näher erklären ? Was spricht gegen einen eigene Klasse ?

asc

Trial schrieb:

erstmal vielen Dank für Eure Tips, die helfen mir prima weiter. Ein paar Fragen habe ich noch:

asc schrieb:

std::tr1::function

Was bedeutet hier tr1 ?

TR1 (Technical Report 1) ist eine Erweiterung zum C++ Standard von 1998.
Für dich wird das aktuell wohl noch nicht von Bedeutung sein, ist nur als Vorgriff zu sehen (Da der nächste Standard so langsam Gestalt annimmt, wird der tr1 in wohl nicht mehr ewiger Zeit auch Bestandteil vom std-Namensraum; Aber das dauert auch noch ein paar Jahre).

Trial schrieb:

asc schrieb:

Vielleicht wäre auch eine std::map noch besser aufgehoben, zumindestens wenn die Fehlernummern "Lücken" haben können.

Und dann die std::exception zu nehmen oder wie meinst Du das ?

Das war bezogen auf dein Array mit den Fehlertexten nicht auf std::exception. Ich halte von den C-Arrays recht wenig (manchmal mögen sie Sinn machen, es gibt aber meist bessere Alternativen).

Du hast derzeit ein Array, aus dem du die Fehlermeldungen über einen Index ausliest. Wenn dieser Index auch Lücken enthalten kann, wäre eine Map besser. So kenne ich z.B. etliche Compiler die "Lücken" bei ihren Fehlernummern haben. std::map ist ein Container mit Einträgen in der Form <Schlüssel, Wert>, hier wäre der Schlüssel die Fehlernummer und der Wert der Fehlertext.

Trial schrieb:

audacia schrieb:

Exception-Klassen sollten mindestens von std::exception ableiten.

Kannst Du mir das näher erklären ? Was spricht gegen einen eigene Klasse ?

1. Wiederverwertung von Code ist meist besser als Neuschreiben
2. Einheitliche Exceptionbehandlung
...

cu André

Vielen Dank für deine Antworten André,

diesen Brocken habe ich aber noch nicht geschluckt:

asc schrieb:

1. Wiederverwertung von Code ist meist besser als Neuschreiben
2. Einheitliche Exceptionbehandlung

Wenn wir mal von Arrays und maps absehen, dann ist "das Neuschreiben" ja kein großer Aufwand, wenn ich jetzt meine paar Zeilen betrachte, und mehr braucht man ja nicht (bis auf die Fehlerbeschreibungen) ?! Und von try/catch her gesehen ist die Behandlung doch auch gleich ?!

Braunstein

Punkt 1 sehe ich auch nicht so kritisch. Punkt 2 aber schon.
Das problem ist hier, wenn du eine Exception fangen willst und Informationen davon benötigst, dann musst du acu den typ der Exception (oder deren basisklasse) in catch mit angeben.
Ein

catch(std::exception& e)

würde aber deine Exceptionklasse nicht fangen. Wenn von std::exception abgeleitet wäre aber schon.

OK, ist mir jetzt klarer was mit Punkt 2 gemeint ist. Ich bin davon ausgegangen, das ich meinen catch Block als ersten nehme und den "std:exception& e" oder die 3 Punkte als 2. hintendran.