Sauberer Code

Kofi

class Konto
{
	public:
		string bankname;
		string kontoinhaber;
		string bezahlungsmethode;
		int    kontonummer;
		int    guthaben;

		Konto::Konto(){
			this->bankname          = "Standartname";
			this->kontoinhaber      = "Sparkes Espanio";
			this->bezahlungsmethode = "Paypal";
			this->kontonummer       = 10120012;
			this->guthaben          = 0;
		}

		int    Konto::getGuthaben()           const { return this->guthaben; }
		int    Konto::getKontonummer()        const { return this->kontonummer; }
		string Konto::getBankname()           const { return this->bankname; }
		string Konto::getKontoinhaber()       const { return this->kontoinhaber; }
		string Konto::getBezahlungsmethode()  const { return this->bezahlungsmethode; }

		void   Konto::KontoInfos(){
			cout << this->getBankname() << endl;
			cout << this->getKontoinhaber() << endl;
			cout << this->getKontonummer() << endl;
			cout << this->getBezahlungsmethode() << endl;
			cout << this->getGuthaben() << endl;
		}

		void   Konto::programmWarten(void){
			cin.clear ();
			cin.ignore();
			cin.get   ();
		}
};

Ich wollte mal eure Meinung hören
Ist es so in Ordnung? sollte ich was dran verändern?
Wie kann ich den Code verbessern?

Jockelx

Hi,

als aller erstes:
'programmWarten' hat in einer Klasse 'Konto' mit Sicherheit nichts zu suchen!

'KontoInfos' ist gernzwertig, aber finde ich ok, auch wenn du wahrscheinlich noch andere Meinungen dazu hören wirst.
(Eine Alternative ist eine freie Funktion, die ein Konto übergeben kriegt und vielleicht einen allgemeinen stream).

Im Konstruktor Initalisierungslisten verwenden.
Und befremdlich finde ich, dass der Default-Konstruktor so konkrete Werte anlegt.

Namenskonventionen nicht einheitlich (einmal (), dann (void), KontoInfos gross, sonst klein)

Woher kennt er string im Header? namespace std global bekannt gemacht?

Gut ist auf jeden Fall, dass die getter const sind.
Edit:
Bzw. doch nicht. Warum getter, wenn die Member alle public sind?
Die sollten (im Zweifel) private sein, dann machen die getter auch wieder Sinn.

(mit aus Bequemlichkeit abgekürzten Bezeichnern)

struct Konto{
  string bank;
  string inhaber;
  string methode;
  int kto;
  int guthaben;
  Konto(void) : bank("..."), inhaber("..."), methode("..."), kto(12345678), guthaben(123){};
  infos(void) const { 
    cout << bank << endl 
     << kto << endl
     /*... usw ...*/
     << guthaben << endl;
  }
  void warten(void) const { /* ... */ }
};

Kofi

Jockelx schrieb:

Hi,

als aller erstes:
'programmWarten' hat in einer Klasse 'Konto' mit Sicherheit nichts zu suchen!

'KontoInfos' ist gernzwertig, aber finde ich ok, auch wenn du wahrscheinlich noch andere Meinungen dazu hören wirst.
(Eine Alternative ist eine freie Funktion, die ein Konto übergeben kriegt und vielleicht einen allgemeinen stream).

Im Konstruktor Initalisierungslisten verwenden.
Und befremdlich finde ich, dass der Default-Konstruktor so konkrete Werte anlegt.

Namenskonventionen nicht einheitlich (einmal (), dann (void), KontoInfos gross, sonst klein)

Woher kennt er string im Header? namespace std global bekannt gemacht?

Gut ist auf jeden Fall, dass die getter const sind.
Edit:
Bzw. doch nicht. Warum getter, wenn die Member alle public sind?
Die sollten (im Zweifel) private sein, dann machen die getter auch wieder Sinn.

Ich habe die Klasse nicht in eine Header Datei geschrieben aber sonst finde ich
deine Kritik gut

Kofi

großbuchstaben schrieb:

(mit aus Bequemlichkeit abgekürzten Bezeichnern)

struct Konto{
  string bank;
  string inhaber;
  string methode;
  int kto;
  int guthaben;
  Konto(void) : bank("..."), inhaber("..."), methode("..."), kto(12345678), guthaben(123){};
  infos(void) const { 
    cout << bank << endl 
     << kto << endl
     /*... usw ...*/
     << guthaben << endl;
  }
  void warten(void) const { /* ... */ }
};

Das ist schwer lesbar

Dieses "this->member" kannst du auch als "member" schreiben

also return guthaben anstatt return this->guthaben

Mechanics

Kofi schrieb:

Ich wollte mal eure Meinung hören
Ist es so in Ordnung?

Ich hasse Denglisch. Ich kann es schon nicht ausstehen, wenn jemand deutsche Bezeichner verwendet, aber dann kommst du auch noch mit gemischten Bezeichnern wie getKontonummer(). Wie kann das sauber sein?
Lass das (void) weg, das braucht man in C++ nicht.
Dass Infos irgendwas über cout ausgibt, finde ich schlecht. Kann man zum Test mal machen, aber das hat in so einer Klasse nichts verloren. Logik und Darstellung muss man strikt trennen.

Th69

Und das "Konto::" ist innerhalb der Klassendefinition auch unnötig (bzw. ist das überhaupt vom Standard so erlaubt?).

Mechanics schrieb:

Lass das (void) weg, das braucht man in C++ nicht.

Mit (void) erkennt man auf einen Blick, ob es sich um eine leere Parameterliste oder um einen Funktionsaufruf mit leerer Argumentliste handelt, und das finde ich vorteilhaft. Ist natürlich Geschmacksfrage.

Mechanics

[quote="großbuchstaben"]

Mechanics schrieb:

Mit (void) erkennt man auf einen Blick, ob es sich um eine leere Parameterliste oder um einen Funktionsaufruf mit leerer Argumentliste handelt

Mach mal ein Beispiel, mir ist spontan nicht ganz klar, wie du das genau meinst.

class A{};
A foo(void){}
A a = foo();

A foo(){}

Da erkennt man aber auch auf einen Blick, dass es sich um eine leere Parameterliste handelt.

Bashar

Das (void) ist ein syntaktischer Spezialfall, den man sich für ANSI-C ausgedacht hat, weil man die Kompatibilität zu K&R bewahren wollte. In C++ sollte man das IMHO nicht verwenden. Das Argument von großbuchstaben finde ich nicht überzeugend. Nichtmal ansatzweise, sorry.

Mechanics

Ich würde die Information, dass es man da dran erkennen könnte, verwirrender finden, als die Erkenntnis, die man dadurch gewinnt.

Ich mach es zwar mittlerweile auch ohne extra void in der Klammer, da ich möglichst reduzierten (auch optisch) Code bevorzuge (sonst krieg ich Krämpfe im Kopf), finde die Konsistenz aber an sich ganz schnicke.

Namenloser324 schrieb:

Ich mach es zwar mittlerweile auch ohne extra void in der Klammer, da ich möglichst reduzierten (auch optisch) Code bevorzuge (sonst krieg ich Krämpfe im Kopf), finde die Konsistenz aber an sich ganz schnicke.

Addendum:
Im Endeffekt aber wieder völlig nichtiges Thema, hauptsache man macht es konsistent.

Tja - da sind wir eben verschieden.

wenn ich foo() in meinem Code sehe, sehe ich sofort, daß das keine Deklaration ist, und wenn ich foo(void) sehe, weiß ich sofort das Gegenteil.

Würde ich für beides "foo()" schreiben, dann bräuchte ich Kontext, bspw., was vor dem "foo()" steht - ein Klassenbezeichner, ein Gleichheitszeichen, ein Operator oder was Anderes.
Das wäre mir eine unnötige Einbuße an Codelokalität - und Codelokalität ist mir wichtig

Also bleibt's beim (void).

volkard

Namenloser324 schrieb:

Namenloser324 schrieb:

Ich mach es zwar mittlerweile auch ohne extra void in der Klammer, da ich möglichst reduzierten (auch optisch) Code bevorzuge (sonst krieg ich Krämpfe im Kopf), finde die Konsistenz aber an sich ganz schnicke.

Addendum:
Im Endeffekt aber wieder völlig nichtiges Thema, hauptsache man macht es konsistent.

nein. sowas nichtiges sollte man schon so machen wie alle anderen.

volkard

Mechanics schrieb:

Ich hasse Denglisch. Ich kann es schon nicht ausstehen, wenn jemand deutsche Bezeichner verwendet, aber dann kommst du auch noch mit gemischten Bezeichnern wie getKontonummer(). Wie kann das sauber sein?

Es ist sauber. Wenn man deutsche Bezeichner nimmt, und dafür gibt es durchaus gute Gründe, dann ist getKotonummer vollkommen ok.

volkard schrieb:

Namenloser324 schrieb:

Namenloser324 schrieb:

Ich mach es zwar mittlerweile auch ohne extra void in der Klammer, da ich möglichst reduzierten (auch optisch) Code bevorzuge (sonst krieg ich Krämpfe im Kopf), finde die Konsistenz aber an sich ganz schnicke.

Addendum:
Im Endeffekt aber wieder völlig nichtiges Thema, hauptsache man macht es konsistent.

nein. sowas nichtiges sollte man schon so machen wie alle anderen.

Hauptsache man macht es konsistent steht da. D.h. arbeitet man in einem Projekt einigt man sich entweder auf eine Variante oder folgt halt der Masse falls du mit deiner Implikation, dass es alle so machen würden, recht hast.
Arbeitet man alleine macht man es halt wie man möchte nur eben konsistent. Seitenweise (Achtung: Übertreibung) drüber zu diskutieren ist halt massive Zeitverschwendung.