Erst Projekt Taschenrechner - verbesserungsvorschläge

icarus2

Du solltest dir umbedingt ein anderes Buch zulegen. Warum kannst du hier nachlesen: http://www.c-plusplus.net/forum/272350

Farcry

Ich fand es bisher nicht schlecht , die beispiele waren zwar nicht die besten aber für mich ist es leicht verständlich und ich komme gut mit diesem Buch klar
Auserdem geht es hier in dem Thread doch garnicht um das Buch.

cooky451

Bis jetzt sieht es zumindest noch nicht all zu schlimm aus.
Aber eine Sache sollte dir doch auffallen: Vergleiche mal die Zeilen 35 und 40 mit 23 und 29.

icarus2

Farcry schrieb:

Ich fand es bisher nicht schlecht , die beispiele waren zwar nicht die besten aber für mich ist es leicht verständlich und ich komme gut mit diesem Buch klar
Auserdem geht es hier in dem Thread doch garnicht um das Buch.

Du findest es vielleicht gut. Als Anfaenger merkt man ja auch nicht, dass man absoluten Muell lernt.
Im Thread steht genug negatives zum Buch.

Skym0sh0

doch ich finde das buch gut für anfänger.

und zwar folgendermassen: man arbeitet ein kapitel aus einem guten buch durch (kA dem primer oder so) und nimmt sich dann das entsprechende kapital aus dem JW buch und streicht solange alle fehler an bis die ganze seite rot ist

Ich habe noch einen super Taschenrechner von Herrn Salomon im Kopf. Wenn er nichts dagegen hat, verlinke ich ihn.

Farcry schrieb:

Ich hoffe ihr könntet mir behilflich sein den Code zu "optimieren" nur ein paar vorschläge wie ich es besser machen könnte , bitte auf anfänger basis

Was hälst du denn hiervon?

#include <iostream>
#include <algorithm>

int main()
{
    using namespace std;

    double zahl1; // int für Division macht nicht das, das du willst. 
    double zahl2;
    const char ops[] = "AaSsDdMm"; // Erlaubte Operationen
    char auswahl;

    cout << "Taschenrechner 0.1 by Farcry\n\n"; // Du musst \n innerhalb einer Zeichenkette nicht in Hochkommas setzen.

    cout << "A = Addition\n";
    cout << "S = Subtration\n";
    cout << "D = Division\n";
    cout << "M = Multiplikation\n";
    cout << "\nBitte geben sie die Rechenart ein: ";
    cin >> auswahl;

    if( find(ops,ops+sizeof(ops),auswahl) == ops+sizeof(ops) ) // Wurde eine erlaubte Operation eigegeben?
    {
        cout << "Fehlerhafte Eingabe";
        return -1;
    }

    cout << "\nBitte geben sie die erste Zahl ein: ";
    cin >> zahl1;

    cout << "\nBitte geben sie die zweite Zahl ein: ";
    cin >> zahl2;

    switch(auswahl) // Für Menüsachen eignet sich doch eine switch-Anweisung besser.
    {
        case 'A':
        case 'a':
            cout << "\nRechnung: " << zahl1 << " + " << zahl2 << " = " << zahl1+zahl2;
            break;

        case 'S':
        case 's':
            cout << "\nRechnung: " <<  zahl1 << " - " << zahl2 << " = " << zahl1-zahl2;
            break;

        case 'D':
        case 'd':
            cout << "\nRechnung: " <<  zahl1 << " / " << zahl2 << " = " << zahl1/zahl2;
            break;

        case 'M':
        case 'm':
            cout << "\nRechnung: " <<  zahl1 << " * " << zahl2 << " = " << zahl1*zahl2;
            break;
    }

    return 0;
}

camper

Gugelmoser schrieb:

Farcry schrieb:

Ich hoffe ihr könntet mir behilflich sein den Code zu "optimieren" nur ein paar vorschläge wie ich es besser machen könnte , bitte auf anfänger basis

Was hälst du denn hiervon?
...

Magische Konstanten die mehrfach auftauchen? Das wäre eine passende Stelle um Aufzählungen einzuführen.

cooky451

Gugelmoser schrieb:

Was hälst du denn hiervon?

Wenn du hier schon Array Hacks mit std::find auspackst, hättest du doch zumindest so etwas schreiben können:

int main()
{
  while (true)
  {
    double var1, var2;
    char operation;
    std::cout << "Please enter calculation: ";
    std::cin >> var1 >> operation >> var2;
    if (std::cin.good())
    {
      switch (operation)
      {
      case '+':
        std::cout << var1 + var2 << '\n';
        break;
      case '-':
        std::cout << var1 - var2 << '\n';
        break;
      case '*':
        std::cout << var1 * var2 << '\n';
        break;
      case '/':
        std::cout << var1 / var2 << '\n';
        break;
      default:
        std::cerr << "Unkown operation: " << operation << '\n';
        break;
      }
    }
    else
    {
      std::cerr << "Input error.\n";
      std::cin.clear();
      std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    }
  }
}

camper schrieb:

Gugelmoser schrieb:
Farcry schrieb:

Ich hoffe ihr könntet mir behilflich sein den Code zu "optimieren" nur ein paar vorschläge wie ich es besser machen könnte , bitte auf anfänger basis

Was hälst du denn hiervon?
...
Magische Konstanten die mehrfach auftauchen? Das wäre eine passende Stelle um Aufzählungen einzuführen.

Du meinst eine Aufzählungen hierfür?

const char ops[] = "AaSsDdMm";

Aber dann muss man doch noch den operator>> überladen für die Eingabe und überprüfen welcher Buchstabe nun eingegebn wurde und ein enum-Objekt entsprechend setzen. Das bläht das Ganze doch voll auf, oder hab hab ich dich nun falsch verstanden, falls ja, kannst dann kurz ein Beispiel anführen :).