A
Nomos schrieb:
So, und was hältst du davon?
Vom ersten Überfliegen her schon besser zu lesen, wenn gleich ich etwas schwächer einrücken würde (sonst wirst du bei ein wenig geschachtelten Ausdrücken Probleme bekommen).
Zumeist schreibt man Bedingungs und Schleifenköpfe in die gleiche Ebene wie den umgebenden Code, und Klammern entweder auf der gleichen Ebene oder mit dem enthaltenen Code eine Ebene tiefer.
// Variante 1
int main
{
cout << "Kleines Einrückbeispiel" << endl;
for(int i=0; i<10; ++i)
{
cout << "Schleifendurchlauf " << i << endl;
}
}
Hier erkennt man auf anhieb das, das erste cout und der Schleifenbegin in einer logischen Ebene liegen, der Schleifendurchlauf liegt selbst eine Ebene tiefer.
// Variante 2 (seltener)
int main
{
cout << "Kleines Einrückbeispiel" << endl;
for(int i=0; i<10; ++i)
{
cout << "Schleifendurchlauf " << i << endl;
}
}
Hier ist der Sinn das auch die Klammern in der Ebene des zugehörigen Codes liegen, ich finde es aber etwas ungewöhnlich zu lesen.
Weitere Varianten sind z.B. die Beginnende Klammer immer noch an das Ende der Vorzeile zu schreiben. Dies ist aber IMHO für Anfänger schlechter zu lesen. Mein eigener Favorit ist Variante 1, bei extrem kurzen Funktionen (die nur aus einer Anweisung bestehen, setze ich die Öffnende Klammer auch mal in die Vorzeile).
So nun ganz kurz zum Inhaltlichen (Näher eingehend nach der Auflistung)
Setze niemals goto in einer Hochsprache ein!
Vermeide viele if()-Blöcke wenn es besser mit else if oder switch geht...
Vermeide zu lange Funktionen, lager dann in eigenständige Funktionen aus...
Weiterhin hatte ich sprechende Namen für Variablen ja schon angemerkt...
Wähle immer passende Typen
Thema: goto
Es gibt viele Gründe gegen goto, grundsätzlich gesagt ist es ein sehr unsauberes Codeelement das vermieden werden sollte (um nicht zu sagen: in Sauberen Programmen wirst du es niemals sehen; ich kenne kein professionelles Programm was goto verwendet). Und du solltest goto niemals einen kompetenten Dozenten oder Arbeitskollegen zeigen...
Man kann goto in der Regel durch umschließende Schleifen ganz einfach ersetzen (Bei dir wäre es eine Schleife die statt dem Menu: Label beginnt und am Ende prüft ob 4 eingegeben wurde, ansonsten wird der Code wiederholt).
Thema: if
Dein Programm führt zwangsweise 4 if-Prüfungen aus, obwohl sich diese wiedersprechen. Bei solchen Werten wie bei dir würde sich hier ein switch deutlich besser machen:
do
{
// ... <-- Menüanzeige
switch(menu)
{
case 1:
// <-- Fall 1; Falls du hier Variablen deklarierst solltest
// du nochmals Klammern wie folgt:
{
// ...
}
break;
case 2:
// <-- Fall 2
break;
case 3:
// <-- Fall 3
break;
}
} while(menu != 4); // Wiederhole solange menu != 4 ist...
Die andere alternative ist wenn du komplexere Ausdrücke verwendest die sich ausschließen mit if / else if / else zu arbeiten, so wird die Auswertung nur solange gemacht bis der zutreffende Fall eintrifft.
do
{
// ... <-- Menüanzeige
if(menu == 1)
{
// ...
}
else if(menu == 2)
{
// ...
}
else if(menu == 3)
{
}
// Ein abschließendes else ist in diesem Fall nicht nötig, da
// wir ja nur die Fälle 1,2,3 brauchen...
} while(menu != 4); // Wiederhole solange menu != 4 ist...
Thema: lange Funktionen
Um so länger eine Funktion ist, umso schwerer ist diese zu lesen. Dann lieber Teile der Funktion wieder auslagern.
// ...
void CheckPrimeNumber(); // Funktionsdeklaration
int main()
{
//...
switch(menu)
{
case 1:
CheckPrimeNumber();
break;
case 2:
// ...
}
void CheckPrimeNumber() // Funktionsdefinition
{
// Hier deinen entsprechenden Code kopieren.
}
Thema: Sprechende Namen
Stell dir einfach vor du hast ein Programm geschrieben und muss ein Halbes Jahr später dieses Programm korrigieren. Mit sprechenden Namen wirst du es deutlich leichter haben es wieder zu verstehen.
Sowas wie:
int a, i, g, no_prime, o=0;
i = 0;
g = 0;
no_prime= 0;
Ist nicht sprechend, zum anderen kannst du auch initialisierungen in eine Zeile schreiben und es gibt mehrere Gründe warum man grade als Anfänger lieber jede Variable in eine einzelne Zeile schreiben sollte...
// Dies ist möglich:
int a=0, i=0, g=0, no_prime=0, o=0;
// Vielleicht ist das aber lesbarer:
int a=0;
int i=0;
// ...
Thema: Typen
Warum hast du no_prime als int definiert, obwohl du nur zwischen ja und nein entscheiden musst?
Besser:
bool no_prime = false;
// Noch besser: (Sprechender)
bool isPrimeNumber = true;
cu André