10
\$\begingroup\$

I'm a freshman Computer Science major looking to build projects for my portfolio. I'm new to C++ and programming in general. Is there any way I can improve this basic calculator I've made? Also, what are the next steps you would recommend considering what I've utilized in the program below? I've got a basic idea of static_cast(), cin.clear(), and cin.ignore(), but I just got the solutions from Stack Overflow.

#include <iostream>
#include <cmath>
using std::string;
using std::cout;
using std::cin;
using std::endl;

int main() {
    float num1 = 0;
    float num2 = 0;
    float total = 0;
    string operation = "a";
    string choice = "a";
        
    do {
        cout << "First Number: ";
        cin >> num1;
        //checks if input is an int
        while (cin.fail()) {
            cin.clear();
            cin.ignore();
            cout << "Please input a number: ";
            cin >> num1;
        }
        num1 = static_cast<float>(num1);
    
        cout << "Operation (+, -, *, /, or ^): ";
        //checks if input is a viable operation
        cin  >> operation;
        while ((operation != "+") && (operation != "-") && (operation != "*") && (operation != "/") && (operation != "^")) {
            cout << "Please input a proper operation: ";
            cin >> operation;
        }
            
        cout << "Second Number: ";
        cin >> num2;
        //checks if input is an int
        while (cin.fail()) {
            cin.clear();
            cin.ignore();
            cout << "Please input a number: ";
            cin >> num2;
        }
        //prevents dividing by zero
        while (num2 == 0 && operation == "/") {
            cout << "You can't divide by zero, silly! Try again: ";
            cin >> num2;
        }
        num2 = static_cast<float>(num2);        

        //does math based on inputted opperation
        if (operation == "+") {
        total = num1 + num2;
        }
        if (operation == "-") {
            total = num1 - num2;
        }
        if (operation == "*") {
            total = num1 * num2;
        }
        if (operation == "/") {
            total = num1 / num2;
        }
        if (operation == "^") {
            total = pow(num1,num2);
        }

        cout << "Total: " << total << endl;

        cout << "Would you like to calculate again? (Y/N): ";
        cin >> choice;

        //same 
        while ((choice != "Y") && (choice != "N") && (choice != "y") && (choice != "n")) {
            cout << "Type a proper response (Y/N): ";
            cin >> choice;
        }

        if ((choice == "N") || (choice == "n")) {
            cout << "Thank you!";
        }
    } while ((choice == "Y") || (choice == "y"));
    return 0;
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ There are 9645 questions tagged C++ (most of 'em answered) here on this site. This is Code Review, so it's a good chance most of the code actually works, and the answers come from "experts" suggesting how the code may be improved... Start reading... When you find a challenge you'd like to try for yourself, ignore the code and answers until after you've built your own... \$\endgroup\$ Commented Jan 17 at 5:07
  • 1
    \$\begingroup\$ Regarding the 'next steps,' please keep in mind that asking about adding new features to your code not a good fit for Code Review. \$\endgroup\$ Commented Jan 17 at 5:34
  • 1
    \$\begingroup\$ A tiny comment that doesn't warrant a review of its own: Your comment says "checks if input is an int", but you're fetching a float, not an int. Comments are supposed to help reading the code, but if they don't match the code, then they make it harder to read. \$\endgroup\$ Commented Jan 17 at 17:00
  • \$\begingroup\$ @CrisLuengo Thanks for the catch. I'd probably change it to "check if input is a number" considering that it accepts both ints and floats. \$\endgroup\$ Commented Jan 17 at 17:32
  • \$\begingroup\$ There is an infinite loop in the code. Do not divide by "0+" \$\endgroup\$ Commented Jan 21 at 18:55

3 Answers 3

Reset to default
3
\$\begingroup\$

General Observations:

Since you are first starting out, it really isn't a bad first effort. You probably haven't learned about software design yet. As another review mentioned you really should be using functions to implement sub-programs.

One of the basic concepts of programming is to keep breaking problems down into smaller and smaller pieces until each problem is very easy to code.

There are multiple design methodologies in computer science, two of them are Object Oriented Design and Top Down Design (sometimes know as Stepwise Refinement). Both of these methodologies break the problem into smaller pieces, they just do it in different ways.

SOLID is 5 object oriented design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.

Some other basic programming principles are:

A Review of the Code

The code doesn't include using namespace std; which is good, but it does have:

using std::string;
using std::cout;
using std::cin;
using std::endl;

The code will more clearly define where cout and other identifiers are coming from (std::cin, std::cout). As you start using namespaces in your code it is better to identify where each function comes from because there may be function name collisions from different namespaces. The identifiercout you may override within your own classes, and you may override the operator << in your own classes as well.

Prefer std::cout << "\n"; over std::cout << std::endl; for performance reasons. std::endl flushes the output which means it calls a system routine. On a time sharing computer this means that the program may be swapped out.

\$\endgroup\$
1
  • \$\begingroup\$ -1 SOLID not applicable at all in this case. DRY and KISS yes but not expanded upon in reference to questions code (shame as much to explore) \$\endgroup\$ Commented Jan 21 at 19:04
7
\$\begingroup\$

Why static_cast?

You already declare num1 and num2 are float type variables. You don't need static_cast here: num1 = static_cast<float>(num1); and num2 = static_cast<float>(num2);.

About Initial Value

In string operation = "a"; and string choice = "a";, why the initial values of operation and choice are set to a? They can be empty value "".

\$\endgroup\$
3
  • 1
    \$\begingroup\$ I static casted num1 and num2 because I found in testing that inputting an integer caused issues, especially with division. Didn't really want to input 1.0, 3.0, or something like that to keep it as a float. \$\endgroup\$ Commented Jan 17 at 17:22
  • 1
    \$\begingroup\$ @RajahWilliams I don't understand that what the issue happened when you input integers. I've tested in Visual Studio environment and there is no problem if you input 1 / 2 = 0.5 \$\endgroup\$ Commented Jan 20 at 9:28
  • \$\begingroup\$ Hmm. Just tested in VSCode and 1 / 2 = 1. I only got 0.5 when I did 1.0 / 2 or 1.0 / 2.0. \$\endgroup\$ Commented Jan 23 at 22:45
7
\$\begingroup\$

Review

The code looks reasonable coming from someone who is setting out learning to code.

The only feedback that comes to mind is, Why not start to write and use a service function?

The copy/paste of several lines of code to obtain both operands should make you pause and ask, "Could I use/write a function to avoid duplicating these lines of code?"

It's early days. You've gotten started. I recommend getting lots of practice writing several different "educational projects" to expand your knowledge and "self-assessment" skills. Continually return to "old projects" to re-write while applying recently acquired techniques and review your own code with your own discerning eye.

Two items arising from this project:

if/else if/else

Switching the already verified 'operation' character would be a good time to experiment with:

if( operation == "+" )
    // add operands
else if( operation == "-" )
    // subtract operands
else if( ...
    ...
else
    // perform final operation

This might lead you to exploring C++'s switch()/case: as an alternative.

Oops!

Kudos for being among the few whose first calculator seeks to prevent division by zero.

Another hazard, though, is trying to calculate results that exceed the range of the variables used in the code. Trying to calculate a very large product could exceed the range of the chosen float variables. Even double has a finite ceiling. (Hint: it will require some "maths skills" to solve this issue. Work out the "how to", then bring that knowledge back to the source code for testing.)

Something for the weekend.
Keep learning!

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.