1

So I tried making a simple calculator that can input 2 floats and do basic operations (addition, subtraction, division, multiplication). This is the first time I'm writing a code without copying the entire thing from a tutorial. I did copy some bug fixes and other things but mostly I did it myself. I want criticism and I want to know what I could've done better.

#include <iostream>
#include <ctype.h>
#include <string>

using namespace std;

char start;
float num1, num2, result;
char op;

int menu(){
    cout<<"\nstart the program? \ny or n:\t";
    cin>>start;
    return 0;
}

int input_number(){
    cout<<"\nenter the first number: ";
    cin >> num1;
    while (!cin){
        cout<<"Error. Number of elements must be numerical. Try again: " << endl;
        cin.clear();
        cin.ignore(256, '\n');  
        cin >> num1;
    }
    cout<<"\nenter the second number ";
    cin >> num2;
    while (!cin){
        cout<<"Error. Number of elements must be numerical. Try again: " << endl;
        cin.clear();
        cin.ignore(256, '\n');  
        cin >> num2;
    }
    return 0;
}

int input_op(){
    cout<<"\nenter which operation you want to execute (+,-,/,*): ";
    cin>>op;
    switch (op)
    {
    case '+': result= num1+num2; break;
    case '-': result= num1-num2; break;
    case '*': result= num1*num2; break;
    case '/': result= num1/num2; break;
    default: cout<<"wrong input";
        break;
    }
    return 0;
}

int main(){
    cout<<"-----Calculator X-----";
    menu();
    if (start=='y' || start== 'Y'){
        input_number();
        input_op();
        cout<<"the result is : "<<result<<'\n';
        menu();
    }
    else{
        cout<<"exiting program";
        return 0;}
}
Rayy21
  • 13
  • 3

2 Answers2

0

  • function don't need to return 0, simply declare those as void return.

    • except int main()

  • the cin>>num; while(!cin){cin>>num;} can be merged as while(!(cin>>num))

  • ignore 256 char may not be enough

    • use std::numeric_limits<std::streamsize>::max() instead

  • for ctype.h, the corresponded c++ header is cctype (relevant SO question)

  • you may actually return the result instead of store in global state.
    • bool menu(), std::pair<int,int> input_number(), char input_op()
  • input_number may get single number instead of 2 (and be called twice)
  • you don't check op is valid (unlike number inputs)
apple apple
  • 10,292
  • 2
  • 16
  • 36
  • I'll try to use void functions, it was annoying to type return 0 every time. I didn't know about (!cin), and I just copied it from another question posted here. same thing with the 256, I dont know what it does, I just copied the whole thing. I'll use the cctype header from now on. ohh thats useful, I am a bit confused with the `std::pair input_number();` how does it work? How do I get two numbers if I call it twice, won't it store it in the same float? I'll work on checking `op` – Rayy21 May 22 '22 at 16:09
  • @Rayy21 function can return a value. so return a pair of `int` means return 2 value. – apple apple May 22 '22 at 16:29
  • @Rayy21 The part of "call it twice" is meat to be when `input_number` return single `int`, so you call it twice to get 2 number. (instead of writing the same code twice with `num1` and `num2`) – apple apple May 22 '22 at 16:31
0

Want to add something to @apple apple said. You don't need to define all the variables in the global scope(start, num1, etc) in this specific case, just use them inside a functions or pass them to a functions when needed. Hope it helped a little :^)

Artem
  • 21
  • 5