C Code Korrekturlesen bitte, besonders Menüstruktur! einfacher "Taschenrechner"

Die oben gezeigte Einrückung wäre in der Tat sehr zu empfehlen. Das mit den Redundanzen stimmt natürlich auch, aber hier noch ein Tipp, wie du sie am besten Auflösen kannst: Eine recht gute Faustregel ist, dass eine Funktion immer nur genau eine Sache machen soll. Wenn eine Funktion zwei Dinge macht, müssten es eigentlich zwei Funktionen sein.

Beispiel:
Deine Funktion Addition() macht im Grunde drei Dinge:
1.) Werte einlesen
2.) Die eigentliche Rechnung durchführen
3.) Benutzer fragen, ob er weiterrechnen will

Wenn du es schaffst, diese drei Schritte sinnvoll in einzelne Funktionen zu packen, löst sich das Redundanz-Problem zeitgleich von selbst.

chmbw schrieb:

aber das mit IF kann ich so veranstalten?

Whitespace (unsichtbare Zeichen) haben in C keinerlei Bedeutung (ausser um Token zu trennen, wie bei i = a + ++b; ). Du kannst einrücken und formatieren wie du willst.

Mikrooptimirer schrieb:

Wenn du es schaffst, diese drei Schritte sinnvoll in einzelne Funktionen zu packen, löst sich das Redundanz-Problem zeitgleich von selbst.

Das klingt sinnvoll.

KlausB

Hallo

noch ein kleiner Tip

schau dir mal switch an (statt der vielen IFs)

Mfg
Klaus

chmbw

herzlichen Dank für die vielen Infos!
ich werde mich gleich mal frisch ans Werk machen und das Ergebnis dann posten

chmbw

Hallo,

hat zwar ein bisschen gedauert, aber jetzt hab ich meinen Code für den "Taschenrechner" nochmal überarbeitet bekommen, hoffe die Einrückungen sind jetzt besser als beim letzten Code :

#include <stdio.h>
#include <stdlib.h>

int Hauptmenue();
int Subtrahieren();
int Addieren();
int Wahl1();
int Weiter();

int main(int argc, char *argv[])
{
  int status=1;

  printf("\tTaschenRechner Version 3.0\n\n");

  while (status != 0)
  {
        status = Hauptmenue();
  }  

 system("PAUSE");	
  return 0;
}

int Hauptmenue()
{
    char wahl;

    printf("Hallo, welche Rechenoperation wollen Sie ausfuehren?\n\n");
    printf("****************************************************\n\n");
    printf("\t[a] - Addition\n");
    printf("\t[b] - Subtrakion\n");
    printf("**************************\n");
    printf("\t[c] - Beenden\n\n");
    printf("Ihre Wahl: ");

    scanf("%c",&wahl);
    getchar();

    switch (wahl)
    {
           case 'A' :
           case 'a' : Addieren(); 
                      break;
           case 'B' :
           case 'b' : Subtrahieren();
                      break;
           case 'C' :
           case 'c' : printf("Auf wiedersehen!\n");
                      return 0;
                      break;
           default  : printf("Falsche Eingabe!\n");
                      return;                        
    }          

    if ( Weiter() == 0)
    {
         return 0;
    }

}                  

int Weiter()
{
    char weiter;

    printf("Weiterrechnen?\n\n");
    scanf("%c",&weiter);
    getchar();

    if (weiter == 'J' || weiter == 'j')
    {
       return 1;
    }
    else
    {
        return 0;
    }
}

int Wahl1(int *a, int *b)
{     
    int z1=0,z2=0;

    printf("Bitte Zahl eingeben: \n\n");

    scanf("%i %i",&z1,&z2);
    getchar();

    *a = z1;
    *b = z2;
}

int Addieren()
{
    int a=0, b=0;

    Wahl1(&a,&b);
    printf("a=%d und b=%d\n\n",a,b);
    printf("Ihr Ergebnis: %i \n\n",a + b);

    return 0;
}                      

int Subtrahieren()
{
    int a=0, b=0;

    Wahl1(&a,&b);
    printf("a=%d und b=%d\n\n",a,b);
    printf("Ihr Ergebnis: %i \n\n",a - b);

    return 0;
}

Das Problem beim letzten Code waren die ganzen Rekursionen, die vielen IFs etc. und ich wollte versuchen möglichst eine Funktion pro "Funktion" bzw. Aufgabe. zu erstellen. Freu mich über jegliche Art konstruktiver Kritik!!!
Danke im Voraus!

Grüße

// statt
scanf("%c",&wahl);
// schreib doch einfach:
wahl = getchar();

// statt
    if (weiter == 'J' || weiter == 'j')
    {
       return 1;
    }
    else
    {
        return 0;
    }
    // schreib einfach:
    return weiter == 'J' || weiter == 'j';
    // (denk mal in Ruhe drüber nach)

Wahl1() geht auch noch einfacher:

int Wahl1(int *a, int *b)
{    
    int z1=0,z2=0;

    printf("Bitte Zahl eingeben: \n\n");

    scanf("%i %i", a, b);
    getchar();
    // hier entweder ein 'return', oder Wahl1() muss 'void' statt 'int' sein
}

case 'c':
               printf("Auf wiedersehen!\n");
               return 0;
               break;    // <-- das ist nutzlos, weil's nach dem 'return'
                         //     nicht erreicht werden kann

Das Problem beim letzten Code waren die ganzen Rekursionen, die vielen IFs etc. und ich wollte versuchen möglichst eine Funktion pro "Funktion" bzw. Aufgabe. zu erstellen.

Ich hab sofort alles verstanden. Das ist oft das wichtigste.
Rekursion ist übrigens ein sehr nützliches Werkzeug. In C muss man zwar ein wenig vorsichtig damit sein, aber lass dir auf keinen Fall das rekursive Denken austreiben.

Ich ja, jetzt kannst du langsam damit anfangen, nachzusehen, was mit dem Ding passiert, wenn man völlig unsinnige Eingaben macht.

Den Aufruf von Weiter() solltest du bei deiner while-Schleife machen, das ist ja auch der Ort, an dem du die Entscheidung brauchst: Dann kannst du einfach "while(Weiter())" schreiben. Addieren() und Subtrahieren() haben immer noch gleichen Code - unterscheiden sich nur im Rechenoperator. Addiere() sollte also wirklich nur so aussehen:

int Addiere(int a, int b)
{
    return a + b;
}

Den Code zur Ein- / Ausgabe verfrachtest du dann am besten in eine eigene Funktion Rechne() in der du auch die switch-Abfrage machst. Statt im switch Funktionen aufzurufen, kannst du natürlich die Rechnung direkt durchführen, ganz wie du willst. Rechne() könnte leicht eine relativ große Funktion werden, also überlege dir was du da vllt. wieder in eigene Funktionen packen könntest - betrachte dabei die einzelnen Schritte der Funktion und was für sich eine abgetrennte Aufgabe darstellt.

Mikrooptimierer schrieb:

Addiere() sollte also wirklich nur so aussehen:
int Addiere(int a, int b)
{
    return a + b;
}

Das ist unter Umständen didaktischer Unsinn. Stattdessen kann man ja gleich a + b schreiben. Ausser du willst jetzt anfangen, mit Funktionszeigern herumzuwerfen. Dann wäre das aber keine schlechte Idee. Nur quält man sich üblicherweise erst durch das Kapitel "Arrays & Zeiger", Funktionszeiger kommen darin immer erst am Ende vor.

int Wahl1(int *a, int *b)
{    
    // z1 und z2 sind natürlich nicht mehr nötig, wenn ich sie nicht verwende
    ...
}

µngbd schrieb:

Stattdessen kann man ja gleich a + b schreiben. Ausser du willst jetzt anfangen, mit Funktionszeigern herumzuwerfen.

Dass man die Rechnung auch direkt ausführen kann, habe ich ja gesagt. Und ja, erst hatte ich tatsächlich eine Antwort verfasst, die Funktionszeiger empfohlen hat, fand es dann aber doch unnötig kompliziert, gerade für eine eigentlich einfache Sache wie den ersten Taschenrechner.

chmbw

Hallo,

herzlichen Dank für eure vielen Antworten, leider bin ich momentan noch in der Uni, deshalb werde ich meinen Code erst heut Abend (hoffentlich) erst überarbeitet posten können und die vielen Tipps genau durchgehen, aber ich hab mir ein paar Gedanken zum return gemacht:

µngbd schrieb:

// statt
    if (weiter == 'J' || weiter == 'j')
    {
       return 1;
    }
    else
    {
        return 0;
    }
    // schreib einfach:
    return weiter == 'J' || weiter == 'j';
    // (denk mal in Ruhe drüber nach)

ich kann also den return-wert ohne Probleme in eine "Wahrheitsabfrage" (kann man das so nennen?!) stecken, sprich wenn die Bedingung unwahr/falsch ist wird 0 returned, sonst !=0, richtig?! somit ist der ganze Spaß doch eigentlich nur eine Kurzform der if-Schleife. Ich hoffe ich habs richtig verstanden?! Wenn ja, find ichs genial Danke!

Grüße

Ich hoffe ich habs richtig verstanden?!

Ja.

chmbw

sooo, nach vielem Tüfteln und überlegen hab ich hier meine aktuelle Version zustande gebracht:

#include <stdio.h>
#include <stdlib.h>

int Hauptmenue();
int Eingabe();
int Addition();
int Subtraktion();
int Weiter();

int main(int argc, char *argv[])
{

  int status = 1;

  printf("\tTaschenRechner Version 3.0\n\n");

  while (status !=0)
  {
        status = Hauptmenue();
  }

  printf("Auf Wiedersehen\n\n");

  system("PAUSE");	
  return 0;
}

int Hauptmenue()
{
    char wahl;
    int ergebnis=0;

    printf("Hallo, welche Rechenoperation wollen Sie ausfuehren?\n\n");
    printf("****************************************************\n\n");
    printf("\t[a] - Addition\n");
    printf("\t[b] - Subtrakion\n");
    printf("**************************\n");
    printf("\t[c] - Beenden\n\n");
    printf("Ihre Wahl: ");

    wahl = getchar();
    getchar();

    switch (wahl)
    {
           case 'a' :
           case 'A' : ergebnis = Addition();
                      printf("Ergebnis: %d\n",ergebnis);
                      break;
           case 'b' :
           case 'B' : ergebnis = Subtraktion();
                      printf("Ergebnis: %d\n",ergebnis);
                      break;
           case 'c' :
           case 'C' : return 0;

           default  : printf("Falsche Eingabe!\n\n");
                      return;
    } 
    return (Weiter());  
}                  

int Eingabe()
{
    int eingabe=0;

    printf("Bitte Zahl eingeben: \n\n");        
    scanf("%i",&eingabe);
    getchar();

    return eingabe;      
}                  

int Addition()
{
    int a=0,b=0;

    a=Eingabe();
    b=Eingabe();

    return (a+b);
}

int Subtraktion ()
{
    int a=0,b=0;

    a=Eingabe();
    b=Eingabe();

    return (a-b);
}

int Weiter()
{
    char weiter='j';

    printf("Weiterrechnen [j] // [n] ?\n\n");
    weiter = getchar();
    getchar();

   return (weiter == 'J' || weiter == 'j');
}

die

getchar();

musste ich bei den Eingaben drunterschreiben, weil ich sonst immer noch ein \n oder so im Puffer hatte, mir viel keine elegantere Lösung ein...Zumindest nehme ich an, dass es der Puffer ist, denn er hat öfters einfach eine Eingabe "angenommen" was halt eigentlich nur noch aus dem Puffer stammen konnte...Außerdem habe ich bewusst versucht, auf Zeiger zu verzichten und bin froh es so hinbekommen zu haben (war für mich echt eine kleine Herausforderung )

Bin auf eure Meinungen sehr gespannt!

Grüße

chmbw schrieb:

Außerdem habe ich bewusst versucht, auf Zeiger zu verzichten

Warum?

chmbw

wenn ich ehrlich bin, just for fun...bzw. um mich ein bisschen intensiver mit den abfolgen der funktionen und den möglichkeiten mit diesen zu beschäftigen

chmbw

fällt keinem mehr was zu dem Code ein, was ich verbessern könnt?

volkard

chmbw schrieb:

fällt keinem mehr was zu dem Code ein, was ich verbessern könnt?

int main(int argc, char *argv[])

Die Parameter würde ich gar nicht angeben, wenn ich sie nicht verwenden will.

{ 

  int status = 1; 

  printf("\tTaschenRechner Version 3.0\n\n"); 

  while (status !=0) 
  { 
        status = Hauptmenue(); 
  } 

  printf("Auf Wiedersehen\n\n"); 

  system("PAUSE");    
  return 0; 
}

Sehe keinen dringenden Bedarf an status.

int main(int argc, char *argv[]) 
{ 
  printf("\tTaschenRechner Version 3.0\n\n"); 

  while (Hauptmenue()) 
		;

  printf("Auf Wiedersehen\n\n"); 

  system("PAUSE");    
  return 0; 
}

case 'a' : 
           case 'A' :

oder

switch (tolower(wahl))

return (Weiter());

Sehe keinen Zweck in den Klammern

return Weiter();

chmbw schrieb:

fällt keinem mehr was zu dem Code ein, was ich verbessern könnt?

mach z.b. die 'main' ganz nach unten, dann musste oben nicht die prototypen hinschreiben (aber ist wohl egal, so gehts ja auch). naja, man könnte das programm bestimmt auf die hälfte verkürzen, aber ich weiss nicht, ob dir das helfen würde. eigentlich ist es ganz ok so (wenn's so läuft wie du willst, ich nehme an, das tut es).

chmbw

danke euch!

;fricky schrieb:

naja, man könnte das programm bestimmt auf die hälfte verkürzen, aber ich weiss nicht, ob dir das helfen würde. eigentlich ist es ganz ok so (wenn's so läuft wie du willst, ich nehme an, das tut es).

Klar möcht ich das so kann ich mir halt ansehen, was ich wie anders schreiben könnte etc., einfach sehen wie es anders geht. Ich weiß ja genau, was passieren soll, von daher ist das mit dem Verstehen auch leichter. Wenn du die Lust dazu hättest, würde ich mich sehr darüber freuen!

Grüße

volkard

chmbw schrieb:

danke euch!

;fricky schrieb:

naja, man könnte das programm bestimmt auf die hälfte verkürzen, aber ich weiss nicht, ob dir das helfen würde. eigentlich ist es ganz ok so (wenn's so läuft wie du willst, ich nehme an, das tut es).

Klar möcht ich das so kann ich mir halt ansehen, was ich wie anders schreiben könnte etc., einfach sehen wie es anders geht. Ich weiß ja genau, was passieren soll, von daher ist das mit dem Verstehen auch leichter. Wenn du die Lust dazu hättest, würde ich mich sehr darüber freuen!

Grüße

Nope!
Das feine Aufdröseln ist total gut für die Übersicht, genau wie Du es sagst. In der Praxis wird man nicht die schlichte Addition auslagern und die anderen arithmetischen Operationen auch nicht. Die auszulagern ist ja auch ein wenig Quatsch. Andererseits hast Du aber die übliche menu-Schleife vereinfacht und die Eingabe(), die mit getchar() den Ärger mit dem überzähligen \n im Eingabestrom wegmacht ist Gold wert.
Der Compiler wird selbsttätig die kleinen Funktionen zurückdröseln. Es ist hier allein die Entscheidung, ob Du selber als Leser dieses Codes mit mehr oder weniger Aufdröselung leben magst. Und wie Du in fünf Jahren den selben Code lesen magst, wenn Du Dich gar nicht mehr an die Erstellung erinnerst. Tendenz: Eher mehr aufgedröselt, aber die arithmetischen Einzelrechnungen vielleicht doch nicht auslagern. Aber Du mußt Dich in alle Richtungen austoben. Nur eigene Erfahrung bringt Dir ein Gefühl für solche Abschätzungen. Das hier war weit aufgedröselt, für mich ein wenig zu weit, aber tolower hat mir sehr gefehlt, das war immerhin ein eigenständiger Begriff, den Du nicht nur einmal oder zweimal verwendet hast und sogar schneller anders geht.
Du solltest mal probieren, wie es sich anfühlt, wenn Du die Operationen nicht auslagerst. Ist es dann in 5 Jahren besser zu lesen? Ist die Menuschleife besser zu lesen, wenn Weiter() nicht ausgelagert ist? Ist Eingabe() gut? Ist ja alles schnell probiert oder bei völliger Eindeutigkeit sich vorgestellt. Ich denkem auf Eingabe() zu verzichten, wäre töricht.

C Code Korrekturlesen bitte, besonders Menüstruktur! einfacher &quot;Taschenrechner&quot;

C Code Korrekturlesen bitte, besonders Menüstruktur! einfacher "Taschenrechner"