4. Programm überprüfen bzgl. Stil

Programm überprüfen bzgl. Stil

  • N

    Hi, ich habe ein ANSI-C-Programm zum umrechnen von römischen Zahlen in arabische Ziffern geschrieben, bin allerdings noch unsicher was den Stil und die Qualität betrifft. Funktionieren tut es aber soweit.

    Würde mich freuen, wenn sich einige von euch Profis mal den Code anschauen könnten und Verbesserungsvorschläge, Empfehlungen oder Kritik äußern könnten. Wäre eine große Hilfe, da ich selber noch nicht hundertprozentig zufrieden mit dem Code bin. Irgendwie denk ich die ganze Zeit, dass es auch einfach gehen muss und ich es unnötig komplex gemacht habe - bzw. unübersichtlich gearbeitet habe.

    Eigentlich wollt ich es noch rekursiv machen, aber ich kann keine römischen Zahlen mehr sehen 😉

    CODE: http://codepad.org/g3J8cqpJ

    Vielen Dank schonmal, ich bin für _jeden_ Hinweis dankbar.
    mfg nor

    ps: habs noch nicht in windows getestet, sollte aber gehen.

    0
  • ?

    Also Java style Klammersetzung in C geht gar nicht. :xmas2:

    Was sonst auffällt, sind die magic numbers für deine Errorcodes. Da besser irgendwelche Bezeichner verwenden (enum, #define oder sowas).

    0
  • ?

    edit:

    Bei Bezeichnern vermischt du munter deutsch und englisch. Besser auf eine Sprache festlegen (vorzugsweise englisch).

    0
  • ?

    edit:

    Das gilt dann natürlich auch für die Kommentare. :xmas2:

    0
  • D

    Die Funktion int roman(int anzahl) ist zu lang und zu kompliziert. Ich wuerde dir Vorschlagen sie zu unterteilen in kleine Funktionen. Eine Funktionen sollte so klein wie moeglich sein und eine Aufgabe erfuellen. Zum Beispiel kannst du den Code

    if( j == 1 || j == 3 || j == 5 ){
    for( tmp = j ; tmp >= 0 ; tmp-- ){
    if( (int)input[i+1] == buchstaben[tmp][0] )
    check = 1;
    }
    if( !check ){
    switch( (int)input[i] ){
    case 86: return -16;
    case 76: return -17;
    case 68: return -18;
    }
    }
    }

    In eine Funktion packen und sie in dem Else-Zweig aufrufen. Dadruch kriegst du auch einen besseren Ueberblick und kannst weitaus besser optimieren.

    Kommentare wie

    // EINGABE UNGÜLTIG - V darf nur einmal vorkommen.

    sind Sinnfrei. Jeder, der C lesen kann, liest das auch aus dem Code

    if( buchstaben[1][2] > 1 ) return -13;

    .

    Solche Kommentare

    // Endergebnis ausrechen und ausgeben

    sind besser, weil sie den Codestueck zusammenfassen, also erklaeren was ein groesseres Codestueck macht.

    Achja, IMHO ist es ganz schlechter Stil Kommentare in der selben Zeile rechts zu schreiben. Man sollte sich doch an die Regel halten, dass die Breite des Codes 80 Zeichen nicht ueberschreiten sollte.
    Also z.B, das ist schlecht:

    if( ! inputValid(anzahl) ) return -1; // input auf fehlerhafte Zeichen überprüfen

    Besser ist:

    // input auf fehlerhafte Zeichen überprüfen
    if( ! inputValid(anzahl) ) return -1;

    0
  • ?

    rox schrieb:

    Also Java style Klammersetzung in C geht gar nicht.

    das schimpft sich 'k&r-style'. kernighan und ritchie haben das verzapft, als gosling et al. noch die wolle aus'm teddybär'n gezupft haben. toll ist es trotzdem nicht.

    DEvent schrieb:

    Die Funktion int roman(int anzahl) ist zu lang und zu kompliziert.

    ich finde eigentlich den ganzen code viel zu kompliziert.
    🙂

    0
  • C

    Dieser Thread wurde von Moderator/in rüdiger aus dem Forum Rund um die Programmierung in das Forum ANSI C verschoben.

    Im Zweifelsfall bitte auch folgende Hinweise beachten:
    C/C++ Forum :: FAQ - Sonstiges :: Wohin mit meiner Frage?

    Dieses Posting wurde automatisch erzeugt.

    0
  • N

    rox schrieb:

    Also Java style Klammersetzung in C geht gar nicht.

    Naja, es gibt viele Arten die Klammern zu setzen, fand diese Methode einen guten Kompromiss aus Platz und Übersichtlichkeit. Welche Arten würdest du empfehlen?

    Btw: Der Code wirkt durch die riesigen Tabulatoren wirklich sehr unübersichtlich, mit 3er-Tabs ist es angenehmer.

    rox schrieb:

    Was sonst auffällt, sind die magic numbers für deine Errorcodes.

    Sollte zumindest beim Betrachen des Codes angenehmer sein, aber macht ihn das nicht unverhältnismäßig lang? Hab ja (wenn auch schlecht) die Fehler noch als Kommentar mit angeben.

    rox schrieb:

    Bei Bezeichnern vermischt du munter deutsch und englisch. Besser auf eine Sprache festlegen (vorzugsweise englisch).

    Stimmt, werde ich anpassen. Muss mal unseren Prof fragen, ob er Englische Kommentare akzeptiert, aber sollte er eigentlich.

    DEvent schrieb:

    Die Funktion int roman(int anzahl) ist zu lang und zu kompliziert. Ich wuerde dir Vorschlagen sie zu unterteilen in kleine Funktionen. Eine Funktionen sollte so klein wie moeglich sein und eine Aufgabe erfuellen.

    Uff, wird zwar stressig, aber werds mal probieren so viel wie möglich auszulagern.
    Aber machen zu viele Funktionen den Code nicht auch unübersichtlich?

    DEvent schrieb:

    Achja, IMHO ist es ganz schlechter Stil Kommentare in der selben Zeile rechts zu schreiben. Man sollte sich doch an die Regel halten, dass die Breite des Codes 80 Zeichen nicht ueberschreiten sollte.

    Hat ich auch mal eine Weile so, aber irgendwie hat mir dann der Platz auf dem Monitor gefehlt, und das scrollen war echt nervig 😉 Denke mal ich muss die Kommentare eh nochmal ausmisten...

    ~fricky schrieb:

    ich finde eigentlich den ganzen code viel zu kompliziert. 🙂

    Was würdest du ändern? Hab ja eingangs schon erwähnt, dass er mir auch nicht wirklich gefällt. Denk halt immer "das muss doch besser und effizienter gehen".

    Danke euch allen erstmal, ich werds morgen mal überarbeiten. Weitere Kritik ist willkommen 🙂
    mfg nor

    0

  • Zeilenumbruch bei 80 Zeichen waere nicht schlecht...

    Ivo

    0
  • ?

    nor schrieb:

    ~fricky schrieb:

    ich finde eigentlich den ganzen code viel zu kompliziert. 🙂

    Was würdest du ändern? Hab ja eingangs schon erwähnt, dass er mir auch nicht wirklich gefällt. Denk halt immer "das muss doch besser und effizienter gehen".

    machs so wie hier: http://wvware.cvs.sourceforge.net/viewvc/wvware/wv/roman.c?revision=1.7
    der code ist kaum länger als deiner, kann aber in beide richtungen umwandeln.
    🙂

    0
  • N

    ~fricky schrieb:

    machs so wie hier: http://wvware.cvs.sourceforge.net/viewvc/wvware/wv/roman.c?revision=1.7
    der code ist kaum länger als deiner, kann aber in beide richtungen umwandeln.
    🙂

    Uih, nein danke. Da sind die Buchstaben hart eingebaut, wollt ich gerade vermeiden.

    Aber ich hab eh festgestellt, dass mein Programm noch einige Fehler hatte.
    Hier ist jetzt die überarbeitete (und hoffentlich korrekt funktionierende) Fassung.

    CODE v2 : http://codepad.org/Gfd3u0wT

    Habe dabei versucht auf konsequente englische Variablen und verständliche Kommentare zu achten.

    Zeilenumbruch ist jetzt auch bei 80 Zeichen 🙂
    Zusätzlich hab ich die Tabulatoren durch Leerzeichen ersetzt, allerdings hab ich ein paar übersehen. Sollte aber insgesamt trotzdem besser zu lesen sein, als die erste Version.

    Die MagicNumbers hab ich gelassen, so komplex ist das Programm ja nicht. Und ich hatte auch keine richtige Idee, wie ich es alternativ lösen könnte.

    Ihr könnte ja nochmal drüber gucken, würd mich freuen.
    nor

    0
  • S

    nor schrieb:

    ~fricky schrieb:

    machs so wie hier: http://wvware.cvs.sourceforge.net/viewvc/wvware/wv/roman.c?revision=1.7
    der code ist kaum länger als deiner, kann aber in beide richtungen umwandeln.
    🙂

    Uih, nein danke. Da sind die Buchstaben hart eingebaut, wollt ich gerade vermeiden.

    hehe, genau darüber wollte ich hier schreiben, dass du lieber 'I' statt 73 schreibst, denn 'I' portabler als 73 ist.

    0
Anmelden zum Antworten