Sauberer Code?



  • Hallo Community.
    Ich bin neu im Forum und auch in der Sprache C.
    Nach lesen von entsprechender Literatur habe ich mit "3-Zeilern" mal versucht das eine oder andere Progrämmchen zu schreiben. Sie funktionierten auch irgendwie.
    Was mir bisher fehlt, ist eine Rückmeldung von erfahrenen Programmierern, ob man den Code besser strukturieren oder eleganter schreiben kann, etc. Wäre es möglich, dass in diesem Forum z.b. mal folgendes winziges Programm kritisiert werden kann. Ich erwarte natürlich nicht, dass sofort geantwortet wird. Solltet ihr der Meinung sein, dass das Forum dafür nicht geeignet ist, akzeptiere ich das auch.
    Ich poste einfach mal ein paar Zeilen zur Berechnung von Primzahlen:

    #include <stdio.h>
    
    int zahl;
    int summe;
    int beginn, ende;
    
    //Eingabe des Zahlenraumes - Funktion
    void eingabe() {
        printf("Zahlenraum bestimmen!\n");
        printf("Anfang: ");
        scanf("%d",&beginn);
        printf("Ende: ");
        scanf("%d",&ende);
    }
    
    //Berechnung, ob mehrere Teiler existieren
    void prim(int vzahl) {
      for(int i = 2; i < vzahl; i++) {
        if(vzahl % i == 0)
          summe++;
      }
      if(summe == 0)
      printf("%d ist eine Primzahl!\n",vzahl);
      summe = 0;
    }
    
    //Hauptfunktion
    int main(void) {
      eingabe();
      for(zahl = beginn; zahl < ende; zahl++) {
        prim(zahl); 
        }
    }
    

    Für einen Kommentar wäre ich sehr dankbar.
    Viele Grüße, Schlemiel.



  • Da die Codetags bei dir anscheinend nicht so richtig wollten:

    schlemiel schrieb:

    #include <stdio.h>
    
    int zahl;
    int summe;
    int beginn, ende;
    
    //Eingabe des Zahlenraumes - Funktion
    void eingabe() {
        printf("Zahlenraum bestimmen!\n");
        printf("Anfang: ");
        scanf("%d",&beginn);
        printf("Ende: ");
        scanf("%d",&ende);
    }
    
    //Berechnung, ob mehrere Teiler existieren
    void prim(int vzahl) {
      for(int i = 2; i < vzahl; i++) {
        if(vzahl % i == 0)
          summe++;
      }
      if(summe == 0)
      printf("%d ist eine Primzahl!\n",vzahl);
      summe = 0;
    }
    
    //Hauptfunktion
    int main(void) {
      eingabe();
      for(zahl = beginn; zahl < ende; zahl++) {
        prim(zahl); 
        }
    }
    

    Globale Variablen gehen gar nicht.

    Die Ausgabe ("%d ist eine Primzahl") sollte nicht in der Testfunktion stehen, sonderm in der aufrufenden Funktion.

    Globale Variablen gehen gar nicht.

    Zu deinem Primzahlentest gibt es ein ppar sachen, die auch du erkennen kannst:
    Den Test auf 2 und gerade Zahl kannst du vorziehen.
    Dannn mach immer Zweierschreitte i+=2 statt i++
    Wenn die Zahl einen ganzzahligen Teiler hat, kannst du den Test (Schleife) doch schon abbrechen.

    Globale Variablen gehen gar nicht.

    Als Schleifenbedingung nimm i*i < vzahl

    Globale Variablen gehen gar nicht.

    unsigned int ist für Primzahlen besser geeignet als signed int

    Globale Variablen gehen gar nicht.



  • Hallo schlemiel,

    irgendwie scheint die Codeformatierung bei deinem Beitrag nicht zu funktionieren, daher hier zum besseren Lesen nochmals:

    #include <stdio.h>
    
    int zahl;
    int summe;
    int beginn, ende;
    
    //Eingabe des Zahlenraumes - Funktion
    void eingabe() {
      printf("Zahlenraum bestimmen!\n");
      printf("Anfang: ");
      scanf("%d",&beginn);
      printf("Ende: ");
      scanf("%d",&ende);
    }
    
    //Berechnung, ob mehrere Teiler existieren
    void prim(int vzahl) {
      for(int i = 2; i < vzahl; i++) {
        if(vzahl % i == 0)
          summe++;
      }
      if(summe == 0)
        printf("%d ist eine Primzahl!\n",vzahl);
      summe = 0;
    }
    
    //Hauptfunktion
    int main(void) {
      eingabe();
      for(zahl = beginn; zahl < ende; zahl++) {
        prim(zahl);
      }
    }
    

    Hier einige Kritikpunkte (du wolltest es ja so haben ;-):
    - auch die Ausgabe (ähnlich wie die Eingabe) solltest du in eine Extra-Funktion packen (EVA-Prinzip)
    - möglichst keine globalen Variablen benutzen, sondern besser lokal oder als Parameter an Funktionen übergeben

    Ansonsten kann ich bei dem kleinen Programm jetzt keine weiteren groben Schnitzer entdecken. Sobald du aber mit Strings hantierst, wirst du eher auf Probleme stoßen.

    P.S. Ist "for(int i = ...)" in neuerem C erlaubt (ich kenne es von C89 noch so, daß man die Variable davor deklarieren musste)? Ich weiß aber vom MS C-Compiler, daß er das auch (wie in C++) durchgehen läßt.



  • Leere Klammern in Funktionsdeklarationen solltest Du vermeiden.

    void eingabe( void ){ /* ... */ }
    


  • Th69 schrieb:

    P.S. Ist "for(int i = ...)" in neuerem C erlaubt (ich kenne es von C89 noch so, daß man die Variable davor deklarieren musste)?

    Seit C99 ja.

    Th69 schrieb:

    Ich weiß aber vom MS C-Compiler, daß er das auch (wie in C++) durchgehen läßt.

    Aber gerade die MS-Compiler sind noch bei C89 stehen geblieben.



  • Hallo, alle Beantworter.
    Vielen Dank erst mal für die schnellen Antworten. Bin echt begeistert über das Engagement, das hier offensichtlich zu hause ist. 🙂

    Würde mich trotz später Stunde nochmal trauen zu fragen:

    DirkB und Th96, warum keine globalen Variablen?

    FurbleWurble, warum keine leeren Klammern in Funktionsdeklarationen?

    Th69, mein gcc-Kompiler unter Linux murrt standardmäßig auch bei der Variablendeklaration innerhalb von For. Erst wenn ich mit Parameter C99 kompiliere ist er zufrieden.


  • Mod

    schlemiel schrieb:

    DirkB und Th96, warum keine globalen Variablen?

    Globale Zustände im Programmen sind die Urzutat jedes unnachvollziehbaren Fehlers. Schon in kleinen Programmen ab 100 Zeilen blickt man kaum mehr durch. Du magst das jetzt vielleicht noch anders sehen, aber das ist wirklich die Todsünde beim Programmieren von funktionierendem und wartbaren Code. Also lern möglichst sofort, wie es richtig geht und verwende globale Variablen niemals* wieder.

    FurbleWurble, warum keine leeren Klammern in Funktionsdeklarationen?

    Weil das in C keine Funktion ohne Argumente bedeutet, sondern eine Funktion mit beliebigen(!) Argumenten.

    Th69, mein gcc-Kompiler unter Linux murrt standardmäßig auch bei der Variablendeklaration innerhalb von For. Erst wenn ich mit Parameter C99 kompiliere ist er zufrieden.

    Und das ist vollkommen richtig so. Wenn sich der Microsoft-Compiler nicht beschwert, dann liegt das höchstwahrscheinlich daran, dass du ihn im C++-Modus benutzt. Tu das nicht! Probier mal etwas aus, was nur in C++ funktioniert, um sicher zu sein. Zum Beispiel:

    class foo {}; int main() {}
    

    *: Es wird vielleicht später mal Momente geben, in denen es nicht anders geht. Aber darauf wirst du erst in Monaten oder Jahren oder nie stoßen. Wenn es so weit ist, wirst du die technische Notwendigkeit erkennen und Bescheid wissen. Faulheit ist übrigens keine technische Notwendigkeit.



  • Furble Wurble schrieb:

    Leere Klammern in Funktionsdeklarationen solltest Du vermeiden.

    void eingabe( void ){ /* ... */ }
    

    Leere Klammern werden als Funktionskörper ausgewertet, somit handelt es sich nicht mehr um eine Deklaration, sondern um eine Definition - eine Neudefinition würde eine Fehlermeldung zur Folge haben.

    DirkB schrieb:

    Globale Variablen gehen gar nicht.

    • Sie sind thread-unsicher.
    • Ihr Name muss in allen Dateien einzigartig sein.
    • Sie machen Code unleserlicher bzw. schwerer wartbar (globaler Scope überladen, Verwirrung mit gleichbenannten lokalen Variablen, gebunden an Funktionen).
    • Sie belegen konstant Speicher, der nicht gebraucht werden könnte.

    Weiteres



  • Vielen Dank nochmal für die ausführlichen Erklärungen und Hinweise!
    🙂



  • Youka schrieb:

    Furble Wurble schrieb:

    Leere Klammern in Funktionsdeklarationen solltest Du vermeiden.

    void eingabe( void ){ /* ... */ }
    

    Leere Klammern werden als Funktionskörper ausgewertet, somit handelt es sich nicht mehr um eine Deklaration, sondern um eine Definition - eine Neudefinition würde eine Fehlermeldung zur Folge haben.

    Gemeint waren die Klammern um die Parameterliste. Wenn die leer sind, bedeutet das, dass die Funktion beliebig viele Argumente aufnehmen kann, als im Wesentlichen dasselbe wie void eingabe(...) . Deshalb sollte man in C lieber void eingabe(void) schreiben.

    (Falls man mal C++ macht, sollte man sich das aber schnell wieder abgewöhnen, da bedeutet () wirklich "leer", so dass (void) so aussieht, als ob man nicht so richtig weiß, was man tut.)


Log in to reply