Lesbarer Quellcode ?

[Artchi]

Hihi, aber du wolltest es ja wissen. :p

Aber du wirst dich die nächsten Jahre verbessern. Ist bei jedem so.

[Michael E.]

Unter aller Sau würde ich das nicht bezeichnen. Es gibt deutlich schlimmeres, glaub mir

[rüdiger]

[quote="SciFi"]

const bool CSupplierCopyGUI::CopySupplier(void)

ok, das const bool als Rückgabetyp sinnlos ist hast du ja selbst erkannt

Allgemein ist die Funktion viel zu lang. Teile Aufgaben lieber in kleine und wohl definierte (== verständlicher Name) Unterfunktionen auf.

CSupplier Supplier(m_lpCore, m_sSource.c_str(), true);

C als Klassen-Prefix ist Oberpfui. (Ok, hört sich so an, als könntest du nicht viel dafür). Auch die API des Konstruktors scheint nicht ideal zu sein, wenn du auf einmal mit c-Strings rumhantieren musst.

CMySQLQuery *lpGroupQuery = new CMySQLQuery(m_lpCore->GetCMySQL());

Nimm für so etwas lieber einen Smart-Pointer, da du sonst Exception-Sicherheit kaum gewährleisten kannst (ich empfehle dir mal ein bisschen in GotW zu lesen.

Ansonsten finde ich dein Namensschema uneindeutig. Mal fangen Variablen mit großen Buchstaben an, manchmal mit kleinen, wobei du dann noch unverstänliche Buchstabenfolgen an den Anfang hängst (gut, ich weiß das soll diese sinnlose UN sein. Aber mal ehrlich. GroupQuery kannst du dir merken und wenn du dir merken kannst, das es lpGroupQuery heißt, dann kannst du dir auch merken das es ein Pointer ist...)

//Gruppen durchlaufen und aendern
	for (int nGroupIndex = 0; nGroupIndex != lpGroupQuery->GetResultCount(); nGroupIndex++)

das sollte vermutlich kein int sein, sondern eher was in Richtung unsigned ...

[SciFi]

rüdiger schrieb:

C als Klassen-Prefix ist Oberpfui.

Ehrlich ? Ich hatte das mal am Anfang so gelernt und seitdem nicht mehr abgelegt.

rüdiger schrieb:

(Ok, hört sich so an, als könntest du nicht viel dafür). Auch die API des Konstruktors scheint nicht ideal zu sein, wenn du auf einmal mit c-Strings rumhantieren musst.

Das ist auf meinen Mist gewachsen. Als ich den Kern geplant habe, wusste ich noch nicht, welche GUI ich nehmen soll. Da jetzt jede Lib Ihre eigenen String hat (QString, wxString) etc und ich den Kern unabhängig haben wollte (reines C++ mit MySQL) habe ich mich für die const char * Variante entschieden. Intern arbeitet die Klasse mit strings.

rüdiger schrieb:

CMySQLQuery *lpGroupQuery = new CMySQLQuery(m_lpCore->GetCMySQL());

Nimm für so etwas lieber einen Smart-Pointer, da du sonst Exception-Sicherheit kaum gewährleisten kannst (ich empfehle dir mal ein bisschen in GotW zu lesen.

Das weiss ich genau was ich mache. Der Zeiger ist gültig, bis sich das Programm beendet. Der Kern (Core) hat die MySQL Klasse als Member.

rüdiger schrieb:

Ansonsten finde ich dein Namensschema uneindeutig. Mal fangen Variablen mit großen Buchstaben an, manchmal mit kleinen, wobei du dann noch unverstänliche Buchstabenfolgen an den Anfang hängst (gut, ich weiß das soll diese sinnlose UN sein. Aber mal ehrlich. GroupQuery kannst du dir merken und wenn du dir merken kannst, das es lpGroupQuery heißt, dann kannst du dir auch merken das es ein Pointer ist...)

Normalerweise fangen bei mir Variablen immer klein an. Und zwar mit der UN Bezeichnung : nIndex etc. Die einzige Ausnahme bilden Klassen. Die fangen immer groß und ohne präfix an.

Das mache ich auch schon seitdem ich C++ programmiere. Früher habe ich mit VB geabeitet und kreuz und quer gearbeitet. Als ich auf C++ umgestiegen bin, wollte ich alles richtig machen UN etc. Nun ist alles Humbug

rüdiger schrieb:

//Gruppen durchlaufen und aendern
	for (int nGroupIndex = 0; nGroupIndex != lpGroupQuery->GetResultCount(); nGroupIndex++)

das sollte vermutlich kein int sein, sondern eher was in Richtung unsigned ...

Da gebe ich Dir recht.

So, nun bin ich genug geprügelt worden

[Marc++us]

SciFi schrieb:

Das weiss ich genau was ich mache. Der Zeiger ist gültig, bis sich das Programm beendet. Der Kern (Core) hat die MySQL Klasse als Member.

Das meint er doch gar nicht.

Die Frage ist, wer lpGroupQuery freigibt, falls die SQL-Abfrage eine Exception wirft, was ja immer mal vorkommen kann.

Btw, ich finde viel schlimmer als die Namenskonvention weiterhin, daß Applikationslogik und Treiberlogik in die GUI gemischt wurde.

[SciFi]

Marc++us schrieb:

SciFi schrieb:

Das weiss ich genau was ich mache. Der Zeiger ist gültig, bis sich das Programm beendet. Der Kern (Core) hat die MySQL Klasse als Member.

Das meint er doch gar nicht.

Die Frage ist, wer lpGroupQuery freigibt, falls die SQL-Abfrage eine Exception wirft, was ja immer mal vorkommen kann.

Btw, ich finde viel schlimmer als die Namenskonvention weiterhin, daß Applikationslogik und Treiberlogik in die GUI gemischt wurde.

Die Klasse wirft keine Execptions. Die ist auch von mir. Ich arbeite mit Rückgabecodes.

[rüdiger]

Das mit dem C am Anfang von Klassennamen hat Microsoft damals für die MFC eingeführt, weil es noch keine Namespaces gab. Damit sollten Namenskonflikte verhindert werden. Aber nun sind dummerweise einige Leute dazu übergegangen das C selbst zu verwenden (aus welchem Grund auch immer, der Lesbarkeit bringt es definitiv nichts oder ist es interessant ob es sich um eine Klasse, Struct, Typedef oder was auch immer handelt?). Das ist nicht sehr sinnvoll.

Ich finde es auch nicht unbedingt sinnvoll Objekte anders zu behandeln als PODs oder Variablen von Builtin-Typen (zB in der Namensgebung). Das zeugt imho eher von einer zumindest antiquierten Verwendung der Objektorientierung.

UN ist wirklich humbug. Das führt zu folgenden Problemen: Du denkst zu viel über den Typ nach, dabei ist eine Variable ja eher ein abstraktes Konzept von etwas. Welcher Typ sich genau dahinter verbirgt ist egal, solange er dem Konzept entspricht (also eine Variable index kann zB vom Typ unsigned, std::size_t, std::vector<T>::size_type, Index etc sein. Das ist im Grunde eher uninteressant).

Außerdem verkompliziert es das programmieren. Du musst wahrscheinlich regelmäßig nachschauen wie die Variable genau heißt (fängt er nun mit einem l oder i oder n etc an?). Solchen Namen kann man sich schlecht merken. Da schaut man lieber mal in dem Fall genau nach, in dem man den Typ genau wissen muss.

SciFi schrieb:

Marc++us schrieb:

SciFi schrieb:

Das weiss ich genau was ich mache. Der Zeiger ist gültig, bis sich das Programm beendet. Der Kern (Core) hat die MySQL Klasse als Member.

Das meint er doch gar nicht.

Die Frage ist, wer lpGroupQuery freigibt, falls die SQL-Abfrage eine Exception wirft, was ja immer mal vorkommen kann.

Btw, ich finde viel schlimmer als die Namenskonvention weiterhin, daß Applikationslogik und Treiberlogik in die GUI gemischt wurde.

Die Klasse wirft keine Execptions. Die ist auch von mir. Ich arbeite mit Rückgabecodes.

Selbst wenn alle Methoden/Funktionen die du aufrufst nie nie nie im Leben eine Exception werfen würden (was ich nicht glaube), enthält dein Code sogar eine Stelle an der eine Exception fliegen kann.

Es ist nicht leicht Exception sicheren Code zu schreiben!

[matimatiker]

Rückgabecodes statt Exceptions sind böse.
Rückgabecodes kann man ignorieren, tut man auch => unauffindbare Fehler entstehen.

Was diese ungarische notation angeht. Was haltet ihr davon diese nicht zu verwenden um den typen zu "dokumentieren", sondern den Verwendungszweck der Variable. Z.b. int bIstAn ? bIstAn kann 0 oder 1 sein, bzw. true oder false?

[rüdiger]

matimatiker schrieb:

Was haltet ihr davon diese nicht zu verwenden um den typen zu "dokumentieren", sondern den Verwendungszweck der Variable. Z.b. int bIstAn ? bIstAn kann 0 oder 1 sein, bzw. true oder false?

Wo ist der Sinn davon? Wenn man eine Variable einschränken will, dann nimmt man eben einen entsprechenden Typ (in dem Fall zB bool) oder bastelt eine entsprechende Klasse. Dafür gibt es ja OOP

[naja]

matimatiker schrieb:

Rückgabecodes statt Exceptions sind böse.
Rückgabecodes kann man ignorieren, tut man auch => unauffindbare Fehler entstehen.

Exceptions kann man in C++ aber auch ignorieren, was das ganze arbeiten damit komplizierter macht.

[matimatiker]

naja ist vorschrift für mich auf der arbeit. habe anfangs auch nicht den sinn davon verstanden. Diese Beispiele sind halt sinnvoller:

int* pInt;
SmartPtr<int> pInt;
int* bufInt;

und kommt mir nicht damit für bufInt std::vector vorzuschlagen. Um z.b. binary-dateien einzulesen ist es nutzlos.

[matimatiker]

naja schrieb:

matimatiker schrieb:

Rückgabecodes statt Exceptions sind böse.
Rückgabecodes kann man ignorieren, tut man auch => unauffindbare Fehler entstehen.

Exceptions kann man in C++ aber auch ignorieren, was das ganze arbeiten damit komplizierter macht.

Ja, aber um die zu ignorieren musst du aufwand betreiben. um rückgabekodes zu ignorien muss du keinen. im gegenteil um auf rückgabecodes zu testen musst du aufwand betreiben.

worauf ich hinaus will?
faulheit

Edit:
Nagut manchmal überwieder der aufwand eine fehler zu beseitigen den aufwand die entsprechende exception zu ignorieren...

[vista]

matimatiker schrieb:

Ja, aber um die zu ignorieren musst du aufwand betreiben. um rückgabekodes zu ignorien muss du keinen.

gute compiler geben ein warning aus, wenn man return values ignoriert...

[matimatiker]

vista schrieb:

matimatiker schrieb:

Ja, aber um die zu ignorieren musst du aufwand betreiben. um rückgabekodes zu ignorien muss du keinen.

gute compiler geben ein warning aus, wenn man return values ignoriert...

nicht alle compiler sind gut. es ist imho nicht sinnvol die warnung standardmäßig auszugeben. vielleicht ist es ja absicht dass ich einen rückgabewert ignoriere.
Anders gesagt der Aufwand ist immer noch da:
* guten compiler finden.
* entsprechende compiler-flag in der doku suchen
* compiler-flag anfügen

[vista]

matimatiker schrieb:

vielleicht ist es ja absicht dass ich einen rückgabewert ignoriere.

dann musst du die funktion so aufrufen

(void)funktion_die_was_zurueckgibt();

[matimatiker]

vista schrieb:

matimatiker schrieb:

vielleicht ist es ja absicht dass ich einen rückgabewert ignoriere.

dann musst du die funktion so aufrufen

(void)funktion_die_was_zurueckgibt();

hmm. wieder was gelernt.

[SciFi]

Ich wollte doch eigentlich nur wissen, ob mein Quellcode verständlich ist

In einem meiner Bücher stand in Vorwort geschrieben, dass jeder Programmierer den Stil des anderen repektieren möchte. Kein Witz. Stand wirklich da. Jeder hat so seine Eigenarten und persönliche Vorlieben. Wenn ich mir mein Programm nach ein paar Monaten wieder anschaue, dann finde ich es sehr hilfreich, wenn die Variablen einen entsprechenden präfix haben. Es liest sich einfach leichter und mann muss nicht immer nachsehen was es nun ist.

Vielleicht sieht einiges auf den ersten Blick komisch aus, aber das Programm besteht aus sehr sehr vielen Programmzeilen und die Logik kann man nicht in einer Funktion darstellen.

Mit Exepctions arbeite ich nicht so gerne. Dieses Try & Catch ist mir irgendwie suspekt. Ich will auch gar nicht, dass das Programm irgendwo rausspringt, sondern unter Berücksichtigung des Fehlers weitermacht. Ich habe vor ein paar Tagen mal einen Thread gelesen, dass viele die Exeption wirklich nur als Notbremse ansehen und nicht zur Standard Fehlerbehebung benutzen. Meine CMySQL kapselt nur die C API von MySQL auf eine bequeme Art und Weise (welche glaube ich gar keine Execption werfen kann) und das sich an den Standard SQL Befehlen etwas ändert glaube ich kaum.

[Marc++us]

SciFi schrieb:

Meine CMySQL kapselt nur die C API von MySQL auf eine bequeme Art und Weise (welche glaube ich gar keine Execption werfen kann)

Tut sie wohl nicht, denn Du schreibst ja immer noch "SELECT" im Klartext in den Quellcode.

[rüdiger]

SciFi schrieb:

Wenn ich mir mein Programm nach ein paar Monaten wieder anschaue, dann finde ich es sehr hilfreich, wenn die Variablen einen entsprechenden präfix haben. Es liest sich einfach leichter und mann muss nicht immer nachsehen was es nun ist.

Wozu musst du immer wissen "was es nun ist"? Es reicht doch wenn du weißt was es tun bzw. darstellen soll.

Mit Exepctions arbeite ich nicht so gerne. Dieses Try & Catch ist mir irgendwie suspekt. Ich will auch gar nicht, dass das Programm irgendwo rausspringt, sondern unter Berücksichtigung des Fehlers weitermacht. Ich habe vor ein paar Tagen mal einen Thread gelesen, dass viele die Exeption wirklich nur als Notbremse ansehen und nicht zur Standard Fehlerbehebung benutzen.

Viele Leute verstehen C++ falsch. C++ ist eben kein "verbessertes C", sondern eine eigene Programmiersprache... Aber das führt jetzt zu weit ab von der Diskussion.

Meine CMySQL kapselt nur die C API von MySQL auf eine bequeme Art und Weise (welche glaube ich gar keine Execption werfen kann) und das sich an den Standard SQL Befehlen etwas ändert glaube ich kaum.

Du hast aber im ganzen wenig Kontrolle darüber wann Exceptions geworfen werden oder nicht, wenn du nicht _alles_ selbst programmierst. Wie gesagt in deiner Funktion gibt es mindestens zwei Stellen an denen Exceptions geworfen werden können. Mindestens eine Stelle davon kann zu einem Ressource-Leak führen (sowohl in der Datenbank(!), als auch in deinem Programm).

Wilde Annahmen über Exception-Verhalten von fremden Funktionen zu treffen _ist_ gefährlich!

[naja]

matimatiker schrieb:

naja schrieb:

matimatiker schrieb:

Rückgabecodes statt Exceptions sind böse.
Rückgabecodes kann man ignorieren, tut man auch => unauffindbare Fehler entstehen.

Exceptions kann man in C++ aber auch ignorieren, was das ganze arbeiten damit komplizierter macht.

Ja, aber um die zu ignorieren musst du aufwand betreiben.

Aufwand? Ich muss doch einfach garnichts machen. Wenn irgend ein netter Kollege auf die Idee kommt ein integer zu werfen, dann kracht es irgendwann mal. Wenn ich in Java eine Methode habe die eine normale Exception wirft, dann muss ich die mit throws angeben, und der Code compiliert nicht, wenn ich sie nicht abfange.