Just made my first program, looking for advice!

[silverwish]

Hi all! I started programming just yesterday and I decided to make a simple calculator, then add to it as I learn more. I've been reading guides and watching videos but there's a few things that are confusing me...here's my calculator, it works perfectly :

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

int main()
{
double firstnum;
double secondnum;
double result;
string selection;
string option[] = { "1", "add", "2", "subtract", "3", "multiply", "4", "divide"};

cout << "Welcome to my Shitty Calculator \n";
start:
cout << "\nDo you want to : \n\n 1) add \n 2) subtract \n 3) multiply \n 4) divide? \n\nPlease enter the number of your choice : ";
cin >> selection;

if (selection == option[0] || selection == option[1]){
goto addition;
}
else if (selection == option[2] || selection == option[3]){
goto subtraction;
}
else if (selection == option[4] || selection == option[5]){
goto multiply;
}
else if (selection == option[6] || selection == option[7]){
goto divide;
}

else if (selection != option[0,1,2,3,4,5,6,7]){
goto error;
}


addition:
cout << "\nType your first number to add : ";
cin >> firstnum;
cout << "Type your second number : ";
cin >> secondnum;
result = firstnum + secondnum;
cout << endl << firstnum << " added to " << secondnum << " is : " << result << endl;

goto start;

subtraction:
cout << "\nType your first number to subtract : ";
cin >> firstnum;
cout << "Type your second number : " << endl;
cin >> secondnum;
result = firstnum - secondnum;
cout << endl << firstnum << " minus " << secondnum << " is : " << result << endl;

goto start;

multiply:
cout << "\nType your first number to multiply : ";
cin >> firstnum;
cout << "Type your second number : ";
cin >> secondnum;
result = firstnum * secondnum;
cout << endl << firstnum << " times by " << secondnum << " is : " << result << endl;

goto start;

divide:
cout << "\nType your first number to divide : ";
cin >> firstnum;
cout << "Type your second number : ";
cin >> secondnum;
result = firstnum / secondnum;
cout << endl << firstnum << " divided by " << secondnum << " is : " << result << endl;

goto start;

error:
cout << endl << "There is no option '" << selection << "' try again..." << endl;
goto start;

return 0;
}


--------------------------------------------------------------------------------

The last "else if" statement I used works just fine (option [1,2,3...), but when I try to use the array the same way for the first "if" (selection == option[1,2]), it doesn't work! It's fine for my program but what if I have 100 options? that's a hell of a lot of copy/pasting, I'm guessing there's a better way.
Also I've been wondering about classes, from what I gather about classes, you use the main file for the body of your program (in my case I guess that's just the option menu and the "if" statements??), and I would use 4 separate classes for my "addition", "subtract", "multiply" and "divide" sections?? If that's right can someone tell me how to separate those sections from my main program? I've read about classes but it doesn't make any sense to me :/ It would be nice to have them separated because I'm going to add graphics and other stuff to my calculator, just for fun ^^.

[rcast]

Here is a tip. Never use goto, 99% of the time there is a better way to achieve the same thing using loops. The use of goto makes what is called spaghetti code, where logic jumps back and forth and is hard to follow. I wouldn't use it anymore, but that's my opinion. Here's an article on it: http://c2.com/cgi/wiki?GotoConsideredHarmful

[Oria]

My advise would be that you use code tags, so that your post is more readable.

However, at a glance I see goto . Please use loops, don't get too familiar with goto , it's bad practice.

[zb0S216C]

I've acknowledged the fact that you're new to C++, so I'll go easy on ya' :)

1) Well done for using "std::string"!
2) Avoid using "goto". Instead, use functions: http://www.cplusplus.com/doc/tutorial/functions/
3) The last "else if" is not doing what you think it should. What you're actually doing is this:

else if( selection != option[7] )
  // ... 

Why? See the section on the comma operator here: http://www.cplusplus.com/doc/tutorial/operators/

silverwish wrote:

It's fine for my program but what if I have 100 options? that's a hell of a lot of copy/pasting, I'm guessing there's a better way.

You need a loop: http://www.cplusplus.com/doc/tutorial/control/ (see the iteration structures section).

silverwish wrote:

"I've read about classes but it doesn't make any sense to me"

http://www.cplusplus.com/doc/tutorial/classes/

...the rest is fine. For a programmer who started just yesterday, you seem to be quite competent in comparison to the other beginners. You're off to a good start :)

For future reference, see here: http://www.cplusplus.com/doc/tutorial/

Oria wrote:

"it's bad practice"

*sigh* No, it's not; it's abused.

_Wazzak

[silverwish]

Alright, thanks guys, I'll have another go at it :)
code tags? As in the "source code" button to the right there?

Framework, thanks for the vote of confidence, and the links ^^
The #include <string> part took me ages to figure out hah

[Yanson]

Please use code tags when you post. There's a menu on the right side of the screen when you post. Code tags is the button that looks like this <>. I makes reading your code easier.

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

int main()
{
	const int ADD = 1;
	const int SUBTRACT = 2;
	const int MULTIPLY = 3;
	const int DIVIDE = 4;
	
	double firstnum;
	double secondnum;
	double result;
	int selection;
	bool loop = true;
	char answer;
	
	cout << "Welcome to my Shitty Calculator \n";

	while (loop)
	{
	
		cout << "\nDo you want to : \n\n 1) add \n 2) subtract \n 3) multiply \n 4) divide? \n\nPlease enter the number of your choice : ";
		cin >> selection;
		cout << "Enter your first number ";
		cin >> firstnum;
		cin.ignore();
		cout << "\nEnter you second number ";
		cin >> secondnum;
		cin.ignore();
		cout << endl;
	
		switch(selection)
		{
		case ADD:
			result = firstnum + secondnum;
			cout << "result " << result << endl;
			break;
		case SUBTRACT:
			result = firstnum - secondnum;
			cout << "result " << result << endl;
			break;
		case MULTIPLY:
			result = firstnum * secondnum;
			cout << "result " << result << endl;
			break;
		case DIVIDE:
			result = firstnum / secondnum;
			cout << "result " << result << endl;
			break;
		default:
			{
				cout << "Invalid entry" << endl;
			}
		}/*end of switch*/

	cout << "Would you like to do another operation? [Y/N]: ";
	cin >> answer;
	cin.ignore();
	answer = toupper(answer);
	if (answer == 'N')
	{
		loop = false;
	}

	}/*end of while loop */

	return 0;
}

Like the others said don't use goto. You don't need it

[silverwish]

Thanks yanson, I see what you're doing there (I think), you've changed the options to integers so you can use switch? (I read somewhere that switch can only use integers) I was using it before but I rewrote my program because I wanted to see if I could make the option menu a bit nicer, that's why I started using strings instead. I want to add in as many features as I can so I can learn better.
I think I see why goto is a bad idea, if the program is massive you would end up jumping around the code a lot, confusing everything, and possibly jumping past useful blocks of code? Maybe breaking your app...or something :)

[Stormhawk]

sigh** Kind of tired of all the no goto talk when most of the people on here don't even know they reason why they were told not to use it. If your learning C++ which you are then there is nothing wrong with learning the goto function and using it. Saying not to use goto is like me tell you all not to write a whole 1000+line program with if statements. the same thing will happen it will quickly become very messy if you abuse it.

The only tip you need silverwish is make more programs. experiment trying to add new things to your calculator taking old things out.

Try using functions in it next. Or making a program that uses functions. Thats the best way to learn. or explore.