Verbesserungsvorschläge für meinen kleinen Rechner

hihacker · 11. Mai 2009

Hy

ich hab mir mal durch ein Tutorial ein bisschen C beigebracht. Jetzt hab ich mal nen kleinen Rechner geschrieben. Bitte sagt mir was gut/schlecht ist und was ihr besser machen würdet.

Code:

#include <stdio.h>

int a, b, c, d=0, e=49, f=49, g=49;
char rzeichen;

int main()              /*Hauptprogramm*/
{
    printf ("%45s","Rechner\n\n");

    printf ("%50s","by hihacker\n\n\n\n\n");

    printf ("%70s","Dies ist ein Rechner fuer die normalen Rechenarten.\n\n\n\n\n");

    while ((e!=110)&&(e!=78)&&(e!=27))              /*Schleife ist aktiv bis n, N oder esc gedrückt wird*/
    {
        printf ("\t\t      Bitte waehlen sie eine Rechenart aus. \n\n\n\t\t      1.Addition\t\t2.Subtraktion\n\n\t\t      3.Multiplikation          4.Division\n\n");

        do          /*Gedrückte Taste wird eingelesen (ohne das Enter gedrückt werden muss)*/
        {
            a=getch();
        }while (a==0);

        if (a==49) /*Unterprogramm zu Addition*/
        {
            printf ("\n1.Addition:\n");

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &b);

            printf ("\nBitte zweite Zahl angeben: ");
            scanf ("%d", &c);

            d = b + c;

            printf ("\n\n%d + %d = %d", b, c, d);
        }

        if (a==50) /*Unterprogramm zu Subtraktion*/
        {
            printf ("\n2.Subtraktion:\n");

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &b);

            printf("\nBitte zweite Zahl angeben: ");
            scanf("%d", &c);

            d = b - c;

            printf ("\n\n%d - %d = %d", b, c, d);
        }

        if (a==51) /*Unterprogramm zu Multiplikation*/
        {
            printf ("\n3.Multiplikation:\n");

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &b);

            printf ("\nBitte zweite Zahl angeben: ");
            scanf ("%d", &c);

            d = b * c;

            printf ("\n\n%d * %d = %d", b, c, d);
        }

        if (a==52) /*Unterprogramm zu Division*/
        {
            printf ("\n4.Division:\n");

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &b);

            printf ("\nBitte zweite Zahl angeben: ");
            scanf ("%d", &c);

            d = b / c;

            printf ("\n\n%d / %d = %d", b, c, d);
        }

        printf ("\n\n\nWollen sie eine weitere Rechnung durchfuehren?  J/N\n\n\n");

        do /*Einlesen ob Programm beendet werden soll*/
        {
            e=getch();
        }while (e==0);

        d = 0; /*Das Ergebnis d wird auf Null zurückgesetzt*/

    }

    return 0;
}

vfl_freak · 11. Mai 2009

Moin,

so auf die Schnelle sind mir drei Dinge "sauer aufgestossen"

:

(a) benutze nie einzelne Buchstaben etc. als Variablennamen, sondern immer 'sprechende' Bezeichner! ! !
Also bspw.: statt "int d" besser "int iResult" oder so, wobei das führenede 'i' bei mir den Datentyp angibt.

(b)

int a, b, c, d=0, e=49, f=49, g=49;

Warum sind a, b und c nicht initialisert ? ? ?
Das ist immer gefährlich!

(c) Ein bißchen mehr Struktur wäre schon schön ;-]
Sowas wie

if (a==51) /*Unterprogramm zu Multiplikation*/

liest sich nur sehr schwer - ich finde es so lesbarer:

Code:

/*Unterprogramm zu Multiplikation*/
if (a==51)
{
    ...
}

Und ein bißchen mehr Kommentar wäre nett, da vieles so kaum nachvollziehbar ist. Unter Umständen weißt Du selber nach ein einigen Monaten nicht mehr, was Du mit einer bestimmten Zeile erreichen wolltest. Das macht eine Fehlersuche dann oft sehr nervig ! !

Gruß
Klaus

hihacker · 11. Mai 2009

Meintest du so ungefähr: (Gut das ich das gemacht habe hab jetzt schon zum Teil nicht mehr gewusst wozu meine Variablen sind

)

Code:

#include <stdio.h>

int rechenart = 0 , zahl1 = 0 , zahl2 = 0, ergebnis = 0 , end = 49;

/*Hauptprogramm*/

int main()
{
    printf ("%45s","Rechner\n\n");

    printf ("%50s","by hihacker\n\n\n\n\n");

    printf ("%70s","Dies ist ein Rechner fuer die normalen Rechenarten.\n\n\n\n\n");

    /*Schleife ist aktiv bis n, N oder esc gedrückt wird*/

    while ((end!=110)&&(end!=78)&&(end!=27))
    {
        printf ("\t\t      Bitte waehlen sie eine Rechenart aus. \n\n\n\t\t      1.Addition\t\t2.Subtraktion\n\n\t\t      3.Multiplikation          4.Division\n\n");

        /*Gedrückte Taste wird eingelesen (ohne das Enter gedrückt werden muss)*/

        do
        {
            rechenart = getch();
        }while (rechenart == 0);

        /*Unterprogramm zu Addition*/

        if (rechenart == 49)
        {
            printf ("\n1.Addition:\n");

            /*Einlesen der ersten Zahl*/

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &zahl1);

            /*Einlesen der zweiten Zahl*/

            printf ("\nBitte zweite Zahl angeben: ");
            scanf ("%d", &zahl2);

            /*Rechnung und Ausgabe des Ergebnises*/

            ergebnis = zahl1 + zahl2;

            printf ("\n\n%d + %d = %d", zahl1, zahl2, ergebnis);
        }

        /*Unterprogramm zu Subtraktion*/

        if (rechenart == 50)
        {
            printf ("\n2.Subtraktion:\n");

            /*Einlesen der ersten Zahl*/

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &zahl1);

            /*Einlesen der zweiten Zahl*/

            printf("\nBitte zweite Zahl angeben: ");
            scanf("%d", &zahl2);

            /*Rechnung und Ausgabe des Ergebnises*/

            ergebnis = zahl1 - zahl2;

            printf ("\n\n%d - %d = %d", zahl1, zahl2, ergebnis);
        }

        /*Unterprogramm zu Multiplikation*/

        if (rechenart == 51)
        {
            printf ("\n3.Multiplikation:\n");

            /*Einlesen der ersten Zahl*/

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &zahl1);

            /*Einlesen der zweiten Zahl*/

            printf ("\nBitte zweite Zahl angeben: ");
            scanf ("%d", &zahl2);

            /*Rechnung und Ausgabe des Ergebnises*/

            ergebnis = zahl1 * zahl2;

            printf ("\n\n%d * %d = %d", zahl1, zahl2, ergebnis);
        }

        /*Unterprogramm zu Division*/

        if (rechenart == 52)
        {
            printf ("\n4.Division:\n");

            /*Einlesen der ersten Zahl*/

            printf ("\nBitte erste Zahl angeben : ");
            scanf ("%d", &zahl1);

            /*Einlesen der zweiten Zahl*/

            printf ("\nBitte zweite Zahl angeben: ");
            scanf ("%d", &zahl2);

            /*Rechnung und Ausgabe des Ergebnises*/

            ergebnis = zahl1 / zahl2;

            printf ("\n\n%d / %d = %d", zahl1, zahl2, ergebnis);
        }

        /*Nachfrage ob Programm beendet werden soll*/

        printf ("\n\n\nWollen sie eine weitere Rechnung durchfuehren?  J/N\n\n\n");

        /*Einlesen ob Programm beendet werden soll*/

        do
        {
            end = getch();
        }while (end == 0);

        /*Das Ergebnis d wird auf Null zurückgesetzt*/

        ergebnis = 0;

    }

    return 0;
}

Sonst noch was zu beanstaten / verbessern?

Hab mal noch das Programm als zip gepackt angehängt.

engelmarkus · 11. Mai 2009

Also wenn ich mal deinen Schreibstil beanstanden darf: Du bist inkonsequent.
Ich persönlich mache es so, dass beispielsweise hier

while ((end!=110)&&(end!=78)&&(end!=27))

Immer Leerzeichen drin sind:

while ((end != 110) && (end != 78) && (end != 27))

Bei dir dagegen ist das einmal so, einmal anders. Außerdem musst du nicht nach jeder Anweisung eine Leerzeile machen. Stattdessen unterteil es doch in logische Blöcke. Du könntest dir z. B. angewöhnen nach jeder sich schließenden geschweiften Klammer eine Leerzeile zu machen.
Außerdem ist es vielleicht etwas platzsparender, wenn du die sich öffnende Klammer immer direkt hinter die Anweisung machst.
Dann sähe ein Teil deines Code so aus:

C:

while ((end != 110) && (end != 78) && (end != 27)) {
   printf("...");

   // Gedrückte Taste wird eingelesen (ohne das Enter gedrückt werden muss)
   do {
      rechenart = getch();
   } while (rechenart == 0);

   // Unterprogramm zu Addition
   if (rechenart == 49) {
      printf("\n1.Addition:\n");
   }
}

Desweiteren kannst du bei "if", "while", usw. immer, wenn du nur eine Anweisung dahinter haben willst, und keinen Block, die Klammern weglassen. Dann ist es aber ganz wichtig, dass du korrekt einrückst

.
Das sind natürlich alles nur Vorschläge.

vfl_freak · 11. Mai 2009

/*Schleife ist aktiv bis n, N oder esc gedrückt wird*/
while( (end!=110) && (end!=78) && (end!=27) )

Und was ist 27, 78, 110 ? ? ?
NIE sogenannte magic numbers verwenden ! ! !

Zudem würde ich die Leerzeile nach den Kommentaren weglassen, um sie eindeutiger zuzuordnen !

gruß
Klaus

hihacker · 11. Mai 2009

Ich werd das mal so machen mit den Leerzeilen und so, aber was soll ich statt den "magic numbers verwenden? Ich hab das nicht anders hinbekommen. Ist ja auch mein erstes wirkliches Programm.

engelmarkus · 11. Mai 2009

while( (end!=110) && (end!=78) && (end!=27) )

schreib lieber

C:

while( (end != 'N') && (end != 'n') && (end != 27) )

Für Esc weiß ich gerade nicht, wie man das macht :-( .
Du könntest aber immerhin eine Konstante definieren und die dann verwenden:

C:

const char ESC = 27;

Matthias Reitinger · 12. Mai 2009

Hallo hihacker,

du hast die Quellcodeabschnitte zur Berechnung des Ergebnisses ja schon "Unterprogramme" genannt. Um die aufgeblasene Main-Funktion etwas zu entschlacken, könntest du nun aus diesen Abschnitten tatsächliche Unterprogramme (also Funktionen) machen. Noch besser wäre es, wenn du dabei gleich noch den duplizierten Code eliminierst (sämtliche Grundrechenarten-Unterprogramme sind bis auf wenige Stellen identisch) und ein einziges, allgemeineres Unterprogramm zur Eingabe, Berechnung und Ausgabe schreibst. Diesem solltest du dann über entsprechende Parameter mitteilen können, welche Rechenart verlangt wird.

Grüße, Matthias

hihacker · 12. Mai 2009

@ Matthias Reitinger
Das hatte ich mir auch schon überlegt und ausprobiert. Ich bin jedoch gescheitert hat nicht so ganz geklappt. Könnt mir vlt jemand nen Lösungsansatz geben oder erklären wie das geht. Ich programmier erst seit ca ner Woche in c, also bitte nicht zuviel Fachbegriffe;-)

Marschal · 12. Mai 2009

So in etwa müsstest du dein Programm strukturieren, wenn du Funktionen, also "Unterprogramme", verwenden möchtest. Dient der Kompfortabilität und der Übersicht und Hanhabung.

C++:

//Includes wie gehabt
#include <bla.h>
//...

// PROTOTYPEN (Funktionsköpfe)
int addieren(int , int);   //wir sagen dem Kompiler, dass er eine Funktion erwarten, die zwei integer bekommt (unsere zu addierenden Zahlen), und einen Integer zurückliefert (unser Ergebnis)
//das geht so weiter...
int subtrahieren(int , int);
//...

//bis hierhin, hast du die Funktionen noch nicht aufgerufen und/oder ausformuliert.

int main ()
{
int m_ia = 0; //member variable_i für Integer, a ist der name
int m_ib = 0; //identisch wie oben

cout<<addieren(m_ia, m_ib); //so rufen wir unsere funktion zum addieren auf und geben die gleich aus.

*Alternativ wäre:
addieren(m_ia, m_ib);
cout<< m_ierg; 
m_ierg muss nur noch deklariert werden...
*/

//weiter im Programm...

return o; //main und Programm ende
}

int addieren(int a, int b) //hier formulieren wir nun unsere Funktion aus 
{ //und sagen, was beim Aufruf geschehen soll
//Direkt im aufruf werden die Variablen aus der Main (also m_ia und m_ib kopiert in die Variablen a und b (passiert direkt beim Aufruf))

return a+b; //gibt der Main Funktion das ergebnis aus a+b wieder.

}

kompiled und getestet

Mfg Dimitrij Borisov

Verbesserungsvorschläge für meinen kleinen Rechner

hihacker

Mitglied

vfl_freak

hihacker

Mitglied

Anhänge

engelmarkus

Erfahrenes Mitglied

vfl_freak

hihacker

Mitglied

engelmarkus

Erfahrenes Mitglied

Matthias Reitinger

ɐɯıǝɹ

hihacker

Mitglied

Marschal

Großer Enumerator

Neue Beiträge