[C++] My First Program xD

[MasterTrainer]

So I have recently started learning C++, and studied some of the basics. Using the basic knowledge I have for functions and parameters, I created this basic one-way calculator that takes two ints, x and y, and based on the user input (add, sub, mult), operates on those two numbers and gives the answer.

Though this only calculates once. I don't know how to make it keep accepting numbers indefinitely, but I would like to find out how.

Code

#include <iostream>


using namespace std;

int calculator( int x, int y, string s);


int main()
{
    int x;
    int y;
    string op;
    cout << "Enter an operation: (maybe mult, add, sub)" << endl;
    getline( cin, op);
    if( op == "add"){
        cout << "You have chosen Addition as your function" << endl;
        cout << "Please enter the two numbers:" << endl;
        cin >> x; cin >> y;
        calculator( x, y, op);
    } else if( op == "sub"){
        cout << "You have chosen Subtraction as your function" << endl;
        cout << "Please enter the two numbers:" << endl;
        cin >> x; cin >> y;
        calculator( x, y, op);
    } else if ( op == "mult"){
        cout << "You have chosen Multiplication as your function" << endl;
        cout << "Please enter the two numbers:" << endl;
        cin >> x; cin >> y;
        calculator( x, y, op);
    } else {
        cout << "You have not entered a valid operation" << endl;
    }

    return 0;
}

int calculator( int x, int y, string s)
{
    int answer;
    if( s == "add"){
        answer = x + y;
        cout << answer << endl;
    } else if( s == "mult"){
        answer = x * y;
        cout << answer << endl;
    } else if (s == "sub") {
        answer = x - y;
        cout << answer << endl;
    }

    return answer;
}

 

[IcemanBo]

You can make an infinite loop with while(0){ } for example.

Put everything into the loop, that has to be repeated.

But I don't think it's very clever to go with all these if statements in your calculation function.

It's not best to make things most complex, just to combine every functionality together into one scope of code.

In addition, substraction, and multiplication it's anyway not needed (because no extra function is needed), but it would be a way better if you would split them into three smaller functions, that get explicitly called in your main, after doing the check.

 

[edo494]

the loop should be while(1), since 0 converts to false, and while(false) will never run it.

Also you are missing #include <string>.

Also to add, if I call calculator(5, 2, "whatever") it will just substract the two values not really important since you are not shipping this, just a reminder.

 

[IcemanBo]

@Edo, heh thanks. Did not think. :d

 

[MasterTrainer]

Thanks IcemanBo and Edo; I will keep that in mind.

So I split up the arithmetic operations into 4 functions (add, sub, mult, and I included div), put the thing in a do while loop, and now it calculates for an indefinite amount of time (yay).

Code

#include <iostream>
#include <string>


using namespace std;


int addition(int x, int y);
int multiplication(int x, int y);
int division(int x, int y);
int subtraction(int x, int y);

int main()
{
    do{
    int x;
    int y;
    string op;
    //Prompt message:
    cout << "Enter an operation: (maybe mult, add, sub, div)" << endl;
    getline( cin, op);
    if ( (op == "add") || (op == "sub") || (op == "mult") || (op == "div")) {
        if (op == "add")
        {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            addition(x, y);
        }
        if (op == "sub")
        {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            subtraction(x, y);
        }
        if (op == "mult")
        {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            multiplication(x, y);
        }
        if (op == "div") {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            division(x, y);
        }
        if ( ! (op == "add") && (op == "sub") && (op == "mult") && (op == "div") ) {

            cout << "Must enter a valid info" << endl;
        }

    }



    }while (1);


    return 0;
}

int addition(int x, int y)
{
    int answer = x + y;
    cout << "The answer is " << answer << endl;
    return answer;
}

int multiplication(int x, int y)
{
    int answer = x * y;
    cout << "The answer is " << answer << endl;
    return answer;
}

int division(int x, int y)
{
    int answer = x / y;
    cout << "The answer is " << answer << endl;
    return answer;
}

int subtraction(int x, int y)
{
    int answer = x - y;
    cout << "The answer is " << answer << endl;
    return answer;
}

But now something weird is going on. The prompt message "Enter an operation" appears twice on the screen from the second iteration onwards. Why does it do that?

 

[chobibo]

Try initializing op to an empty string ("").
Nevermind, It didn't work either, sorry.

EDIT:

int main()
{
    do{
    int x;
    int y;
    string op;
    flush(cout); // <--

 

[MasterTrainer]

Try initializing op to an empty string ("").
Nevermind, It didn't work either, sorry.

EDIT:

int main()
{
    do{
    int x;
    int y;
    string op;
    flush(cout); // <--

What does flush(cout); do?

Anyways, I thought of an idea: what if I used a bool to check if the message is shown? If it's true don't show it anymore and if it is false, print the message. Voila! who knew that this was the answer

code

#include <iostream>
#include <string>


using namespace std;


int addition(int x, int y);
int multiplication(int x, int y);
int division(int x, int y);
int subtraction(int x, int y);

int main()
{
    do{
    int x;
    int y;
    bool IsMessageShown;
    string op;

    if ( IsMessageShown == false){
        cout << "Enter an operation: (maybe mult, add, sub, div)" << endl;
        IsMessageShown = true;
    }
    cin >> op;
    if ( (op == "add") || (op == "sub") || (op == "mult") || (op == "div")) {
        if (op == "add")
        {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            addition(x, y);
            IsMessageShown = false;
        }
        if (op == "sub")
        {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            subtraction(x, y);
            IsMessageShown = false;
        }
        if (op == "mult")
        {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            multiplication(x, y);
            IsMessageShown = false;
        }
        if (op == "div") {
            cout << "Enter two numbers:" << endl;
            cin >> x; cin >> y;
            division(x, y);
            IsMessageShown = false;
        }

    } else if (( op != "add") || ( op != "sub") || ( op != "mult") || (op != "div")){
            cout << "Please enter a valid operation" << endl;

    }



    }while (1);


    return 0;
}

int addition(int x, int y)
{
    int answer = x + y;
    cout << "The answer is " << answer << endl;
    return answer;
}

int multiplication(int x, int y)
{
    int answer = x * y;
    cout << "The answer is " << answer << endl;
    return answer;
}

int division(int x, int y)
{
    int answer = x / y;
    cout << "The answer is " << answer << endl;
    return answer;
}

int subtraction(int x, int y)
{
    int answer = x - y;
    cout << "The answer is " << answer << endl;
    return answer;
}

 

[chobibo]

I think it clears the output stream... Not sure though.

 

[edo494]

http://en.cppreference.com/w/cpp/io/manip/flush

 

[Dr Super Good]

Looks like a procedural coupling mess...

#include <iostream>
#include <string>


using namespace std;

int main()
{
    do{
    int x;
    int y;
    int answer;
    bool IsMessageShown;
    string op;

    if ( IsMessageShown == false){
        cout << "Enter an operation: (maybe mult, add, sub, div)" << endl;
        IsMessageShown = true;
    }
    cin >> op;
    if ( (op == "add") || (op == "sub") || (op == "mult") || (op == "div")) {
        cout << "Enter two numbers:" << endl;
	cin >> x; cin >> y;
        if (op == "add") answer = x + y;
        if (op == "sub") answer = x - y;
        if (op == "mult") answer = x * y;
        if (op == "div") answer = x / y;
        cout << "The answer is " << answer << endl;
        IsMessageShown = false;

    } else if (( op != "add") || ( op != "sub") || ( op != "mult") || (op != "div")){
            cout << "Please enter a valid operation" << endl;

    }



    }while (1);


    return 0;
}

This should do exactly the same thing as yours.

 

[edo494]

DSG your code triggers Undefined Behaviour, so you better fix it

 

[Dr Super Good]

Oh yes, however that was not my error. I purely was re-arranging the logic to couple less procedurally and assumed the logic was already correct enough.

Just so you know, the problem is...

    bool IsMessageShown;

You have failed to initialize it before this line occurs...

 if ( IsMessageShown == false){

The C++ standard does not define initial values for locals resulting in "Undefined Behaviour". The reason it works (if it really does work) is probably because of how the stack is allocated when the thread is initialized resulting in the appropriate value. However this is not reliable (can vary with compiler and platform) and so you should explicitly initialize it to the appropriate value.

 

[edo494]

the mainstream compilers will actually not let you compile code with not initialized locals anymore. Also locals can be intialized e.g. static(still local), only with automatic lifetime are not initialized

 

[Nestharus]

If you want to improve it so that you can do something like this

3 + 4*16/(4 - 1)^6

You might wanna try implementing this -> http://en.wikipedia.org/wiki/Shunting-yard_algorithm

That is the way calculators will do it

Alternatively, you could try out some Java, C#, or Python with Antlr4 ; ), then it becomes pretty easy because the parse tree it generates will follow the correct order of operations for you when you visit it ;o.