Tipps zum Programmierstil

Also für mich sieht das im Großen und Ganzen in Ordnung aus. Das Buch von Herrn Breymann ist auch wirklich ein tolles Buch. Das ist immer meine absolut erste Wahl.

Vielleicht noch ein paar Dinge, die ich anders machen würde (sollte aber auch im Buch in einem späteren Kapitel noch erwähnt werden):

Vor diese kurzen Funktionen wie

int Automat::get_preis()
{
    return preis;
}

kannst du das Wort "inline" schreiben. Das ist eine Empfehlung für den Compiler, statt die Funktion aufzurufen einfach den Code in der Funktion zu "kopieren". Der Compiler nimmt das aber wie gesagt nur als Empfehlung. Du zwingst ihn damit nicht.

Längere Zeilen wie deine Ausgabe hier

std::cout << "Person " << id << " kommt mit " << muenzen << " Muenzen zum Automaten.\n";

Kann man zur besseren Lesbarkeit auch auf mehrere Zeilen aufteilen. In diesem Fall könnte man zwar einfach die "<<" in die neue Zeile setzen

int main()
{
    std::cout << "Das "
              << "ist "
              << "ein "
              << "Test"
              << std::endl;
    return 0;
}

aber ganz allgemein kannst du Code in mehrere Zeilen trennen, indem du am Ende der Zeile ein einzelnes Backslash ("\") machst.

Aber wie gesagt, das grundliegende stimmt und die Feinheiten kommen, wenn du weiterliest

Gruß Jörg

Eisflamme

JoergK:
Ich würde beiden Vorschlägen nicht zustimmen. Der Compiler erkennt das schon ganz alleine, eigentlich wird inline sowieso einfach ignoriert.

Und aus einer Zeile fünf zu machen, na ja, Geschmackssache...

Eisflamme schrieb:

JoergK:
Ich würde beiden Vorschlägen nicht zustimmen. Der Compiler erkennt das schon ganz alleine, eigentlich wird inline sowieso einfach ignoriert.

Und aus einer Zeile fünf zu machen, na ja, Geschmackssache...

Also inline benutz ich da wo ich es für richtig halte, aber was ich so mitbekommen habe macht das ja ohnehin jeder anders.

Die 5 Zeilen waren oben nur ein Beispiel für die Verwendung - sinnvoll ist das in meinem Beispiel natürlich nicht. Nur aus dieser einen längeren Zeile, die ich vom Threadsteller zitiert hätte, da hätt' ich vielleicht 2 draus gemacht, ums der Länge der anderen etwas anzupasen.

SeppJ schrieb:

Ein paar Punke:
- Gewöhne dir const-correctness an. Deine Methoden sollten klar machen, ob sie ein Objekt (auch das eigene!) verändern oder nicht. Gucke dir an, was const in welchem Zusammenhang alles bedeuten kann. Leichte Warnung: Das ist am Anfang nicht einfach. Außerdem kann man es nicht halbrichtig machen. Setzt man ein const, so ist dieses ansteckend und man muss es überall richtig machen. Aber dafür kann der Compiler später viel mehr Logikfehler finden als ohne und dieser Vorteil ist gewaltig.
Das heißt übrigens nicht, jede popelige Variable const zu machen! Mache Memberfunktionen ggf. const und zeige bei Pointer- und Referenzparametern, ob das dahinterliegende Objekt verändert wird oder nicht.
- Ein Pfad wie "../include/" vor Includes ist ungewöhnlich. Standardincludepfade sollten über Compileroptionen gesetzt werden, nicht absolut angegeben werden. Hier gehören nur Pfade relativ zu den Standardsuchpfaden hin.

Okay, ist angekommen und werde ich ändern :). (Ich glaube das Thema "sicheres Programmieren" kommt sowieso noch im Buch vor.

SeppJ schrieb:

Alteingesessene Forenmitleser wissen, dass dies wohl das höchste Lob ist, welches von mir jemals auf solch eine Frage als Antwort kam . So drückt man das eben in Westfalen aus. Ein Drei-Sterne-Koch bekommt da auch nur ein "Kann man essen" zu hören

Dann werde ich mich wohl mal glücklich schätzen

JoergK schrieb:

Vor diese kurzen Funktionen wie
int Automat::get_preis()
{
    return preis;
}
kannst du das Wort "inline" schreiben. Das ist eine Empfehlung für den Compiler, statt die Funktion aufzurufen einfach den Code in der Funktion zu "kopieren". Der Compiler nimmt das aber wie gesagt nur als Empfehlung. Du zwingst ihn damit nicht.

Ja, ich hab auch darüber nachgedacht, aber dann hätte ich alle Methoden als inline markiert und habs mir deshalb mal gespart. Ist das eigentlich egal, oder bringt das wirklich etwas? Ich hab gelesen, die meisten Compiler arbeiten sowieso selbstständig in der Sache.

Danke für Eure Tipps

SeppJ

JoergK schrieb:

Eisflamme schrieb:

JoergK:
Ich würde beiden Vorschlägen nicht zustimmen. Der Compiler erkennt das schon ganz alleine, eigentlich wird inline sowieso einfach ignoriert.

Und aus einer Zeile fünf zu machen, na ja, Geschmackssache...

Also inline benutz ich da wo ich es für richtig halte, aber was ich so mitbekommen habe macht das ja ohnehin jeder anders.

Das inline an der Stelle ist aber sinnlos. inline wird heutzutage eigentlich nur wegen seine "Nebeneffekte" benutzt, da es Funktionsdefinitionen in Headern ermöglicht. Und es gibt einen Grund, warum man diesen Nebeneffekt hat: Bis vor wenigen Jahren (siehe unten) war es technisch notwendig, dass dem Compiler die Funktionsdefinition auch tatsächlich vorliegt, damit er die Definition direkt im Code einbauen kann. Das heißt bei Funktionsdefinitionen in separaten Übersetzungseinheiten besteht sowieso keine Möglichkeit, dass der Compiler den Code überhaupt inlined. Außer bei Benutzung der Funktion in der gleichen Übersetzungseinheit. Aber dann sagt Eisflamme schon richtig, dass ein optimierender Compiler sich dann sowieso sein eigenes Bild macht und inline oder nicht komplett ignoriert.

Diese Erklärung ist semi-veraltet. Es sind ein paar Techniken aufgekommen, einen weiteren Optimierungsschritt (oder die gesamte Optimierung) zur Linkzeit durchzuführen. Dann kann der Optimierer auch durch Übersetzungseinheiten hindurchgucken. Das ist aber nicht der übliche Weg der Übersetzung und muss normalerweise explizit aktiviert werden und ist meiner Meinung nach teilweise noch etwas frickelig (wird aber schnell besser).

Wenn es also wirklich auf Inlinecalls ankommt (hier ist dies wohl kaum der Fall und sollte nicht gemacht werden), dann sollte man die Funktion auch so definieren, dass die Definition an den entscheidenden Stellen für den Optimierungsschritt sichtbar ist. (D.h. in der Regel inline im Header. Man beachte dass Funktionsdefinitionen in einer Klassendefinition implizit inline sind).

Also die ganzen Ausgaben mit std::cout gehören nicht in die Klasse Person!

styleq schrieb:

Also die ganzen Ausgaben mit std::cout gehören nicht in die Klasse Person!

Warum sollten sie nicht in meine Klasse dürfen? Verstehe dein Problem dabei nicht. Die Klassen sind ausschließlich zum Üben mit Programmierung auf Kommandozeilenbasis, also werden sie später auch nicht für GUI-Programmierung benötigt (sowieso nicht :p).

Cyres

Ich denke, worauf styleq hinauswollte, ist eine strikte Trennung von Anzeige, Speicherung und Agierungsebene, kurz: MVC-Pattern - Model-View-Controller (ein Punkt, worauf das Buch afair nicht explizit eingeht, aber drauf hinarbeitet, dass ich das Buch gelesen habe ist auch schon einige Donnerstage her ), aber mach dir darüber fürs erste erstmal keine Gedanken. Gerade um zu wissen, was der Compiler eigentlich gerade mit deinem Code anstellt sind anfangs Ausgaben auf der Konsole sehr hilfreich, bzw. auch, um zu verstehe, wie C++ eigentlich arbeitet, wann z.B. ein Copy-Constructor aufgerufen wird und wann der Assign-Operator (=) seinen Nutzen findet. Wenn man sowas nicht einmal über die Konsole ausgibt, wird es schwer, es rein theoretisch zu verstehen.

Allgemein kann ich mich allerdings den Vorpostern anschließen, große Mankos im Stil sehe ich jetzt nicht.

Oberon_0

Das Einzige was mir noch auffällt ist eine inkonsistente Benennung der Methoden:
muenzen_einwerfen, knopf_druecken: objekt_verb
entnehme_rest: verb_objekt (vs. rest_entnehmen)
ist_gesperrt: ist-Präfix; genug_muenzen: kein Präfix (hat_genug_muenzen?)

Eisflamme

Vielleicht gibt es da auch Gegenargumente, aber ich finde, dass man _ in einer case-sensitiven Sprache nicht benötigt, sondern _x zu X konvertiert werden sollte (außer in Wörtern, die rein in Majuskeln geschrieben sind). Finde das schöner lesbar.

Cyres

Ein Gegenargument wäre, dass sowohl die STL als auch Boost beide die _-Schreibweise benutzen und man im Endeffekt die Schreibweisen mischt. Allerdings kann man so besser eigene Dinge erkennen, ich benutze eigentlich auch immer lower-CamelCase für Funktionen und Variablen und Upper-CamelCase für Klassen u.Ä.

daddy_felix

Noch eine Kleinigkeit: in der "Person.h" brauchst du nicht das "#include Automat.h". Es genügen in diesem forward-declarations, weil du "Automat" nur als Referenz verwendest. Das "#include Automat" muss nur in "Person.cpp".

Dsa ist in diesem Fall relativ unwichtig, aber wenn du ein größeres Projekt hast, dann kann die Änderung in einer Headerdatei dazu führen, dass sehr viele andere Dateien kompiliert werden. Da ist es aus zeitlichen Aspekten schon sehr hilfreich, wenn in Header-Dateien möglichst wenige includes stehen.

pumuckl

Drei Dinge die mir aufgefallen sind:

Zum Teil überflüssige Kommentare:

dass das Ding mit dem "~" davor ein Destruktor ist weiß man. Da noch extra einen Kommentar dranzuschreiben "//Destruktor" ist etwas zu viel des Guten. Du schreibst ja auch nicht überall dran "das ist eine Variable", "das ist eine Funktion", ... Ein "Kategorie"-Kommentar wie bei den Aktivitäts- und Abruf-Funktionen würde reichen (Konstruktoren, Destruktor, op=)
Kommentare, die etwas ganz offensichtliches beschreiben, sind imo auch überflüssig: "// Gibt "true" zurück, wenn die Person genug Muenzen hat" - hey, die Funktion heißt schon "genug_muenzen", oder? Genauso "// Muenzen werden eingeworfen". Du hast netterweise die Funktionsnamen schon ausführlich genug gewählt, dass die meisten Kommentare überflüssig werden.

Zum Teil zu kurze Namen: die Parameter deiner Funktionen heißen a, m etc. Das ist deutlich zu kurz, benenne sie so, dass man weiß, was dahinter steckt und nicht erst suchen muss. Automat(int d, int m); Hier kann ich aus dem Header nicht erkennen, was die beiden ints bedeuten, die ich dem Automaten übergebe.
Zum Teil fehlende Kommentare: bei beiden Klassen sind der op= und der Copy-Ctor nicht vorhanden. Wenn man sich die Klassen genauer anschaut, erkennt man, dass die Compilergenerierten da auch völlig ausreichend sind, dennoch wäre es noch besser, wenn du bei den anderen Konstruktoren einen kurzen Kommentar hinterlässt, dass du sie bewusst weggelassen hast. Damit man auf den ersten Blick sieht, dass sie nicht vergessen wurden.