Is This Simple Calculator Coded Correctly? C++

[f.ben.isaac]

1. Simple calculator - nothing fancy

/**program description:
 *
 * simple calculator written for beginners. 
 * This program asks user to enter an arithmetic
 * expression that contains two numbers and one 
 * operator. User will be given the result of the 
 * calculation. Operators allowed to be used 
 * are: + - * / % ...
 *
 * Author: X-Shab
 * Copyright 2008 - 2009
 *
 */

#include<iostream>
#include<string>
using namespace std;

const string EXIT = "-1";

void add(float num1, float num2)
{
	cout<<"Result is: "<< num1 + num2 <<endl;
}

void subtract(float num1, float num2)
{
	cout<<"Result is: "<< num1 - num2 <<endl;
}

void multiply(float num1, float num2)
{
	cout<<"Result is: "<< num1 * num2 <<endl;
}

void divide(float numerator, float denominator)
{
	if (denominator != 0)
		cout<<"Result is: "<< numerator / denominator <<endl;
	else
		cout<<"You can not divide by denominator\n";
}

void modulus(float num1, float num2)
{
	cout<<"Result is: "<<  (int)num1 % (int)num2 <<endl;
}

int main(void)
{
	string mathematicalOperation;
	float num1, num2;
	char operatorSign;
	
	cout << "Please enter an arithmetic expression (-1 to Exit)\n";
	cin >> mathematicalOperation;

	//search for operator sign and perform calculation
	while(mathematicalOperation.compare(EXIT))
	{
		int getFirstDigitIndex = mathematicalOperation.find_first_of("0123456789");
		int operatorSignIndex = mathematicalOperation.find_first_of("+-*/%", getFirstDigitIndex);
		
		if (operatorSignIndex != -1)
		{	
			operatorSign = mathematicalOperation.at(operatorSignIndex);
		
			string firstNumber = mathematicalOperation.substr(0,operatorSignIndex);
			string secondNumber = mathematicalOperation.substr(operatorSignIndex + 1);

			//convert numbers from string to float
			num1 = (float)atof(firstNumber.c_str());
			num2 = (float)atof(secondNumber.c_str());

			switch(operatorSign)
			{
				case '+':
					add(num1,num2);
					break;
				case '-':
					subtract(num1,num2);
					break;
				case '*':
					multiply(num1,num2);
					break;
				case '/':
					divide(num1,num2);
					break;
				case '%':
					modulus(num1,num2);
					break;
			}
		}
		
		cout<<"Please Enter Another Expression or Enter -1 to Exit:\n";
		cin>>mathematicalOperation;
	}

	cout<<"SEE YOU LATER!\n";

	return 0;
}

2. Simple calculator using function template

/**program description:
 *
 * simple calculator written for intermediates. 
 * This program asks user to enter an arithmetic
 * expression that contains two numbers and one 
 * operator. User will be given the result of the 
 * calculation. Operators allowed to be used 
 * are: + - * / % ... 
 *
 * Author: X-Shab
 * Copyright 2008 - 2009
 *
 */

#include<iostream>
#include<string>
using namespace std;

const string EXIT = "-1";

template <class T>
void performCalculation(T num1, T num2, const char operatorSign)
{
	switch(operatorSign)
	{
		case '+':
			cout<<"Result is: "<< num1 + num2 <<endl;
			break;
		case '-':
			cout<<"Result is: "<< num1 - num2 <<endl;
			break;
		case '*':
			cout<<"Result is: "<< num1 * num2 <<endl;
			break;
		case '/':
			if (num2 != 0)
				cout<<"Result is: "<< num1 / num2 <<endl;
			else
				cout<<"You Can Not Divide by Zero\n";
			break;
		case '%':
			cout<<"Result is: "<<  (int)num1 % (int)num2 <<endl;
			break;
		default:
			cout<<"wrong operator sign"<<endl;
	}
}

int main(void)
{
	string mathematicalOperation;
	float num1, num2;
	char operatorSign;
	
	cout << "Please enter an arithmetic expression (-1 to Exit)\n";
	cin >> mathematicalOperation;

	while(mathematicalOperation.compare(EXIT))
	{
		int getFirstDigitIndex = mathematicalOperation.find_first_of("0123456789");
		int operatorSignIndex = mathematicalOperation.find_first_of("+-*/%", getFirstDigitIndex);
		
		if (operatorSignIndex != -1)
		{	
			operatorSign = mathematicalOperation.at(operatorSignIndex);
		
			string firstNumber = mathematicalOperation.substr(0,operatorSignIndex);
			string secondNumber = mathematicalOperation.substr(operatorSignIndex + 1);

			//convert numbers from string to float
			num1 = (float)atof(firstNumber.c_str());
			num2 = (float)atof(secondNumber.c_str());

			performCalculation(num1, num2, operatorSign);

		}
		
		cout<<"Please Enter Another Expression or Enter -1 to Exit:\n";
		cin>>mathematicalOperation;
	}

	cout<<"SEE YOU LATER!\n";

	return 0;
}

1. Is the template in the second example is structured and used correctly?
2. I tried to make my code easy to understand by putting descriptive names. Do you find it easy to follow? Is the code structure well written?

[adam1122]

I didn't analyze it to offer my full opinion but one thing I quickly noticed: % is a form of division and you need to take the same precautions you would with the the / operator.

The structure looks pretty good.

Include a space between #include and <iostream>

[adam1122]

Also, you should print an error if the input is bad.

[f.ben.isaac]

Thanks

[nucleon]

"Please criticize my code." My favorite question! :)

The biggest problem is that your variable names are too long. "Descriptive" does not mean "long". Here are some better names:

mathematicalOperation   expr       (or expression, if you must)
operatorSign            oper       (or operation)
getFirstDigitIndex      iDigit
operatorSignIndex       iSign
firstNumber             strNum1    notice that these go nicely
secondNumber            strNum2    with the names num1 and num2

Here are the structural changes I would make:

int main() {
    string expr;
    float num1, num2;
    char oper;
    while (1) {
        cout << "Enter an expression (-1 to exit)\n";
        cin >> expr;
        if (expr == "-1") break;
        int iDigit = ...;
        int iSign = ...;
        if (iSign == -1) {
            cout << "No operator found.\n";
            continue;
        }
        // etc.
    }
}

Get rid of the "SEE YOU LATER" at the end. (Why do beginners always do that?)

Do not put copyright notices in simple code like this (or anything you write for the next four years).

[f.ben.isaac]

Get rid of the "SEE YOU LATER" at the end. (Why do beginners always do that?)

Do not put copyright notices in simple code like this (or anything you write for the next four years).

The copyright is in there by mistake i guess :-)

Get rid of the "SEE YOU LATER" at the end. (Why do beginners always do that?)

:-)) Now, you made my day

[f.ben.isaac]

I found that template function should be called as

performCalculation<float>(num1, num2, operatorSign); even though
performCalculation(num1, num2, operatorSign); worked fine

Regards

[VernonDozier]

Get rid of the "SEE YOU LATER" at the end. (Why do beginners always do that?)

For some reason, I like stuff like that.

Do not put copyright notices in simple code like this (or anything you write for the next four years).

It's never too early to start claiming your code. People steal beginners' code all the time. For example, one of my earliest programs was:

#include <iostream>
using namespace std;

int main ()
{
     cout << "Hello world!" << endl;
     return 0;
}

I've seen this program (verbatim) hundreds of times since I wrote it, so I can only assume that people have stolen my code, and I have not received a penny in royalties from it. If I had copyrighted it, I'd be a rich man now.

[mvmalderen]

I think you should also do some form of syntax checking ...
Check what happens if you input 56 + 5 ? (with very much spaces between '56' and '+' ... And with very much spaces between '+' and '5') ...
You could make a function which reads out all the spaces (and characters between the operand and the operandus)
Or, you could just give an error ...

You have to decide it !

BTW, Nice program ! :)

[mvmalderen]

Include a space between #include and <iostream>

The space isn't really needed, the code will also compile without a space between #include and <iostream>

[adam1122]

The space isn't really needed, the code will also compile without a space between #include and <iostream>

It was a suggestion for readability. It's minor but the space should be there as far as I'm considered. But it's just my opinion.

[mvmalderen]

It was a suggestion for readability. It's minor but the space should be there as far as I'm considered. But it's just my opinion.

Actually it doesn't make much sense as it's his choice to write it with or without a space :) ...

[adam1122]

Actually it doesn't make much sense as it's his choice to write it with or without a space :) ...

Of course it is but when given a choice people often make a bad one ;)

I'm not trying to make a big stand on this but I would say this is one of those things that just make sense to do and I thought I would encourage it.

[f.ben.isaac]

I think you should also do some form of syntax checking ...
Check what happens if you input 56 + 5 ? (with very much spaces between '56' and '+' ... And with very much spaces between '+' and '5') ...
You could make a function which reads out all the spaces (and characters between the operand and the operandus)
Or, you could just give an error ...

You have to decide it !

BTW, Nice program ! :)

I agree with you, i should have done input validation. However, i will leave it as an exercise for those who wants to practice :-P

[f.ben.isaac]

You should not put any spaces
between the arithmetic expression, it should
be written as 5+3 or -6+3, and so on

[mvmalderen]

Of course it is but when given a choice people often make a bad one ;)

I'm not trying to make a big stand on this but I would say this is one of those things that just make sense to do and I thought I would encourage it.

In this case he can't make a bad choice ...

And here stops the discussion !

[mvmalderen]

You should not put any spaces
between the arithmetic expression, it should
be written as 5+3 or -6+3, and so on

Yeah, agreed, but you're expecting the user is always inputting valid information, without ever making a typo, so it might be better to do some input validation and display an error in that case ...

[adam1122]

In this case he can't make a bad choice ...

And here stops the discussion !

I disagree.

[mvmalderen]

I disagree.

Why it's just a useless discussion, and what's the main topic?
"a space" ...

BTW, What wrong choice could he make in this case ?
NO ONE !
Because the C++ preprocessor is just ignoring that space ...

[f.ben.isaac]

Putting a space between #include and HEADER NAME makes the code looks better as far as i see. On the other hand, i respect tux4life point of view too. Thanks to all who participated in this thread

Thread Solved

[adam1122]

Why it's just a useless discussion, and what's the main topic?
"a space" ...

BTW, What wrong choice could he make in this case ?
NO ONE !
Because the C++ preprocessor is just ignoring that space ...

I guess it's more of a principle thing for me. All other things being equal, if one thing has benefits over the other (even if they are very small), you should choose that thing.

I just find when I stray from this, even on small things, I am more likely to stray from it with bigger things. That's why it's so important to me. But really not a big deal in this discussion.

Now if you said there are benefits to no space, I would be more likely to accept it as being ok.

[mvmalderen]

I guess it's more of a principle thing for me. All other things being equal, if one thing has benefits over the other (even if they are very small), you should choose that thing.

I just find when I stray from this, even on small things, I am more likely to stray from it with bigger things. That's why it's so important to me. But really not a big deal in this discussion.

Now if you said there are benefits to no space, I would be more likely to accept it as being ok.

If you don't type a space, your code will be 1 character shorter, less typing :P ...
But actually both don't have real benefits, it's only a matter of preference ...