C/C++ Forum :: Bankautomat. Kritik erwünscht.

Unregistrierter

Hallo,

Nachdem ich nun einige Tage sowohl hier als auch im IRC genervt habe, möchte ich nun endlich mal ein Ergebnis zeigen.

Zur Information:
Ich arbeite seit knapp einer Woche mit c++, dies ist mein erstes Programm, wenn man von Tests der Ein und Ausgabefunktionen absieht. Ich habe das bankautomaten Thema gewählt als ich davon hier gelesen habe:

http://www.c-plusplus.de/forum/viewtopic-var-t-is- ....... days-is-0-and-postorder-is-asc-and-start-is-0.html

Ich hoffe auf (nicht allzu harte) Kritik und Verbesserungsvorschläge und auch darauf irgendjemandem damit weiterhelfen zu können und sei es nur als schlechtes Beispiel.

Hier der Quelltext:

http://codepad.org/yWdyvRar

Und hier die verwendeten 3 Textdateien:

http://rapidshare.com/files/297793450/Automat.rar
||
http://ul.to/nd2yke

freakC++ Mitglied 20:18:56 25.10.2009 Titel:

Du solltest nicht so viele gloable Variablen verwenden. Das kann man schöner lösen. Versuche möglichst lokal zu bleiben. Da du noch ein Anfänger bist, schätze ich, dass Du noch keine Klassen kennst. Sonst könnte man daraus eine wunderbare Klasse machen.

lg, freakC++

Kóyaánasqatsi Mitglied 20:25:19 25.10.2009 Titel:

Och für eine Woche finde ich das vollkommen OK! Zwar sind die Einrückungen an manchen Stellen echt pervers, aber für einen ersten Anfang finde ich das (verhältnissmäßig) ordentlich.

Registrierter Troll Mitglied 20:25:50 25.10.2009 Titel:

Ich habs nur kurz überflogen.
Was mir sofort aufgefallen ist, ist der switch-Abschnitt ganz am Anfang. Dort steht drei mal exakt der gleiche Code. Warum nimmst du nicht die Kontonummer als Dateinamen und sparst dir die Unterscheidung im Code? Dann ist es auch ganz einfach um weitere Konten zu erweitern.

Auch der restliche Code ist teilweise mehrfach vorhanden - lagere doch das Lesen der Kontodaten in eine eigene Funktion aus, die du an den richtigen Stellen aufrufst, dann wird der Code gleich viel schlanker.

SeppJ Moderator 20:31:02 25.10.2009 Titel:

Wie freakC++ schon sagte, weniger globale Variablen. Versuch es mal ganz ohne globale Variablen.

Das feste Einbauen der vorhandenen Konten im Programm ist ziemlich unelegant, könntest du die Kontoinformationen aus einer Datei lesen?

Einigen deiner Variablennamen könnten ein paar Vokale gut tun. Lieber etwas mehr Schreibarbeit auf sich nehmen, wenn es dadurch verständlicher wird.

Etwas Schreibarbeit sparen, könntest du dir, indem du dir mal anguckst, was die Operatoren +=, -=, *= und /= machen.

kontoabf in Zeile 231 wird nirgends benutzt.

abheben und ueberweisen haben als Rückgabewert int, geben aber nix zurück.

Da beim Aufruf von abheben und ueberweisen der Rückgabewert nicht verwendet wird, vermute ich mal, dass du eigentlich void als Rückgabewert haben willst.

Ein guter Compiler kann dich übrigens auf manche dieser Sachen schon hinweisen, du musst ihm nur beibringen, dass er etwas mitteilsamer sein soll.

ipsec Mitglied 20:35:14 25.10.2009 Titel:

Ich persönlich finde die Kommentare alá

C/C++ Code:
/* ========================= Kommentar ========================= */

C/C++ Code:
/* ========================= Kommentar ========================= */

C/C++ Code:
/* ========================= Kommentar ========================= */

innerhalb einer Funktion ungünstig, da die Funktion dadurch stark unterbrochen wird, die Übersicht leidet und infolge dessen das Lesen erschwert wird. Ich präferiere

C/C++ Code:
// Kommentar

C/C++ Code:
// Kommentar

C/C++ Code:
// Kommentar

Unregistrierter

Vielen Dank für das bisherige Feedback

Dass die beiden Funktionen abheben und überweisen vom Typ int sind, hat den Grund dass ich es ursprünglich so konzipiert hatte sie die Kundennummer zurückgeben zu lassen, was sich dann aber als schwachsinnig herausgestellt hat.

Als Compiler verwende ich g++.

Der Vorschlag, das Lesen und Schreiben in eine eigenständige Funktion zu packen ist super und wird sofort umgesetzt.

Wo liegt das Problem der globalen Variablen, ich dachte dabei an den Umstand sie nicht für jeden Block neu zuweißen zu müssen.

freakC++ Mitglied 21:08:45 25.10.2009 Titel:

Was heißt neuzuweisen? Du kannst Parameter verwenden oder mit Zeiger / Referenzen arbeiten (wobei Du das wahrscheinlich nocht nicht kennst).

Natürlich sind globale Variablen nicht "böse", aber es ist ein schlechter Stil, wenn der Programmierer aus Bequemlichkeit alle Variablen einfach als global deklariert.

Diese werden nämlich erst beim Programmende zerstört, es kann zu Namenskonflikten kommen (da man bei einem Programm immer ein bestimmtes Namensschema hat) und nicht die Variablen wahllos benennt.

Gewöhne dir es gleich wieder ab, nur globale Variablen zu verwenden. Ich hatte leider ein nicht so gutes Buch, weshalb dieses Forum mir letzendlich den ständigen Gebrauch von globalen Variablen wieder herausgeprügelt hat

lg, freakC++

Nexus Mitglied 21:59:56 25.10.2009 Titel:

freakC++ schrieb:

Natürlich sind globale Variablen nicht "böse", aber es ist ein schlechter Stil, wenn der Programmierer aus Bequemlichkeit alle Variablen einfach als global deklariert.

Sie können durchaus böse sein, schliesslich hat der schlechte Stil auch einen Grund. Meist zeigen sich die Auswirkungen aber erst in grösseren Projekten und sind Anfängern daher nicht bewusst.

Globale Variablen verleiten dazu, sich keine Überlegungen mehr über modulare Programmierung und Zugehörigkeit der einzelnen Programmbestandteile zu machen. Entsprechend kann das gesamte Design darunter leiden. Eine Lösung nur aus Bequemlichkeit zu bevorzugen ist meist eine schlechte Idee. Die Vorteile sind nur von kurzer Dauer, hingegen schlagen die Probleme erst mit der Zeit hinterrücks zu.

Zu diesen gehört in erster Linie die grosse Abhängigkeit, die zwischen globalen Variablen und den deren Nutzern entstehen. Das wirkt sich einerseits negativ auf die Kompilierzeit aus, andererseits verletzt es die Kapselung. Da der Zugriff auf globale Variablen von überall (zumindest in den inkludierenden Dateien) her erfolgen kann, wird es schwierig, die Benutzung zurückzuverfolgen und Fehler einzuschränken. Je weniger Informationen eine Klasse oder Funktion über ihre Umgebung hat, desto weniger Seiteneffekte kann sie haben. Durch ein strukturiertes Design bleiben die Aufgabenbereiche lokal und sauber voneinander getrennt, was die Wartung erleichtert, da beispielsweise schnell ersichtlich ist, wo eine Änderung durchgeführt werden muss. Potenzielle Fehler haben zudem eine grössere Chance, an Ort und Stelle behandelt werden zu können.

freakC++ Mitglied 22:46:08 25.10.2009 Titel:

Ich wollte mit "böse" sagen, dass es durchaus manchmal angebracht eine globale Variable zu definieren. Dann sind sie nützlich, doch sollte dies nicht zur Gewohnheit werden, denn dann werden sie "böse".

lg, freakC++

Nexus Mitglied 23:10:26 25.10.2009 Titel:

freakC++ schrieb:

Ich wollte mit "böse" sagen, dass es durchaus manchmal angebracht eine globale Variable zu definieren. Dann sind sie nützlich, doch sollte dies nicht zur Gewohnheit werden, denn dann werden sie "böse".

Da hast du natürlich Recht. In .cpp-Dateien (sodass nur lokale Funktionen auf sie zugreifen dürfen) können globale Variablen zum Beispiel sinnvoll sein.

Ich dachte jedoch, wabbul und andere Leute, die diesen Thread lesen, hätten nichts gegen eine etwas ausführlichere Begründung als nur "schlechter Stil".

Th69 Mitglied 15:34:08 26.10.2009 Titel:

Zitat:

Bankautomat. Kritik erwünscht.

Ich vertraue mein Geld nie wieder einer Bank an -)

Nein, im Ernst, für erst eine Woche C++ Programmiererfahrung sieht das schon recht gut aus. Detaillierte Kritik wurde ja schon geäußert, der ich mich auch anschließe (keine globalen Variablen, eigenartige Kommentar-Einrückung, Auslagerung in Funktionen).

Sehr positiv finde ich, daß du gleich von Anfang an die C++ Datentypen (std::string, std::iostream etc.) benutzt (du scheinst ein gutes Buch zu benutzen).
Wenn du bald noch die Erstellung von Klassen lernst und dein Programm darauf umbaust, dann bist du auf einen guten Weg, um auch andere Projekte in Angriff zu nehmen. Viel Erfolg noch und besonders Spaß dabei!