Sign in to follow this  
HeyHoHey

question on looping + general clarity of code

Recommended Posts

HeyHoHey    122
okay well i am making a small program to try and use what i have learned. i have multiplication division etc. i have it looped with a do while loop if the user selects something wrong.(not one of the choices 1-5) I want to know how i would get it to reloop after they chose lets say addition and added numbers. what kind of loop would i use and how would i implement it. also i would like the user to have a choice, for instance something saying would you like to have me do more math for you? then it loops back the program. also if anyone has any comments on if my code is sloppy or how i can improve it i will be greatful. thanks hey heres the code #include <iostream> using namespace std; int multiply(int a, int b); int addition(int x, int y); float division(float q, float t); int subtraction(int z, int c); main() { int choice; /* addition ints */ int x; int y; /* multiplication ints */ int a; int b; /* division ints */ float q; float t; /* subtraction ints */ int z; int c; do { cout<<"Would you like me to do multiplication, division, addition, or subtraction for you?\n"; cout<<"Choose 1-5 from the following list.\n"; cout<<"1. Addition\n"; cout<<"2. Subtraction\n"; cout<<"3. Multiplication\n"; cout<<"4. Division\n"; cout<<"5. Exit\n"; cin>> choice; switch (choice) { case 1: cout<<"You have selected Addition.\n"; cout<<"Type in the first number.\n"; cin>> x; cin.ignore(); cout<<"Type in the second number.\n"; cin>> y; cin.ignore(); cout<<"When you add " << x <<" and " << y << " you get " << addition(x, y) << ".\n"; cin.get(); break; case 2: cout<<"You have selected Subtraction.\n"; cout<<"Type in the first number.\n"; cin>> z; cin.ignore(); cout<<"Type in the second number.\n"; cin>> c; cin.ignore(); cout<<"When you subtract " << z <<" from " << c << " you get " << subtraction(z, c) << ".\n"; cin.get(); break; case 3: cout<<"You have selected Multiplication.\n"; cout<<"Type in the first number.\n"; cin>> a; cin.ignore(); cout<<"Type in the second number.\n"; cin>> b; cin.ignore(); cout<<"When you multiply " << a <<" and " << b << " you get " << multiply(a, b) << ".\n"; cin.get(); break; case 4: cout<<"You have selected Division.\n"; cout<<"Type in the first number.\n"; cin>> q; cin.ignore(); cout<<"Type in the second number.\n"; cin>> t; cin.ignore(); cout<<"When you divide " << q <<" by "<< t << " you get " << division(q, t) << ".\n"; cin.get(); break; case 5: cout<<"You have chosen to quit. Have a nice day and goodbye!\n"; cout<<"Hit enter one more time to exit.\n"; cin.get(); break; } } /* loop for if the user picks anything besides 1-5 */ while (choice != 1 && choice != 2 && choice != 3 && choice != 4 && choice != 5); } int addition(int x, int y) { return x + y; } int multiply(int a, int b) { return a * b; } float division(float q, float t) { return q / t; } int subtraction(int z, int c) { return z - c; }

Share this post


Link to post
Share on other sites
ToohrVyk    1595
Two main problems with your code:
  • Your functions (division, substraction etc) provide no useful abstraction. Remove them and replace them with the operators.
  • You should define any variables as close as possible to the place where they are used. Like, just before you read a value from the user.


You would also benefit from abstracting the entire "read from user, perform operation, display message" into a single function, and passing the operations and strings as arguments to that function depending on usage.

You could also cleanly rewrite your loop condition as a single inequality, if you used unsigned integers to read the option.

Share this post


Link to post
Share on other sites
HeyHoHey    122
Quote:
Original post by ToohrVyk
Two main problems with your code:
  • Your functions (division, substraction etc) provide no useful abstraction. Remove them and replace them with the operators.
  • You should define any variables as close as possible to the place where they are used. Like, just before you read a value from the user.


You would also benefit from abstracting the entire "read from user, perform operation, display message" into a single function, and passing the operations and strings as arguments to that function depending on usage.

You could also cleanly rewrite your loop condition as a single inequality, if you used unsigned integers to read the option.


sorry i forgot to mention im new to c++ so i dont know all the lingo completely :P

Your functions (division, substraction etc) provide no useful abstraction. Remove them and replace them with the operators.

i dont understand that, what do you mean by abstraction and what do you mean by replace them with operators.

i understand the second one to define variables as close to where it is used as possible. is this because it takes less memory or make it run a little quicker? or is it just proper format for coding?

also i did not understand

You would also benefit from abstracting the entire "read from user, perform operation, display message" into a single function, and passing the operations and strings as arguments to that function depending on usage.


thanks
heyho

Share this post


Link to post
Share on other sites
raptorstrike    181
By abstraction he means "make more flexible" or provide a layer of ambiguity to your code (if someone has a better definition by all means post it).

You would also benefit from abstracting the entire "read from user, perform operation, display message" into a single function, and passing the operations and strings as arguments to that function depending on usage.

much of the code here repeats itself and the volume of code could be cut down considerably if the output was put in a function. Here is your code as it stand (slightly edited to get rid of operator functions and other ints)


#include <iostream>

using namespace std;

//int multiply(int a, int b);
//int addition(int x, int y);
//float division(float q, float t);
//int subtraction(int z, int c);

main()
{
int choice;
// integers on which operation will be performed
int x, y;

do
{
cout<<"Would you like me to do multiplication, division, addition, or subtraction for you?\n";
cout<<"Choose 1-5 from the following list.\n";
cout<<"1. Addition\n";
cout<<"2. Subtraction\n";
cout<<"3. Multiplication\n";
cout<<"4. Division\n";
cout<<"5. Exit\n";
cin>> choice;

switch (choice)
{
case 1:
cout<<"You have selected Addition.\n";
cout<<"Type in the first number.\n";
cin>> x;
cin.ignore();
cout<<"Type in the second number.\n";
cin>> y;
cin.ignore();
cout<<"When you add " << x <<" and " << y << " you get " << x + y << ".\n";
cin.get();


break;
case 2:
cout<<"You have selected Subtraction.\n";
cout<<"Type in the first number.\n";
cin>> x;
cin.ignore();
cout<<"Type in the second number.\n";
cin>> y;
cin.ignore();
cout<<"When you subtract " << x <<" from " << y << " you get " << x - y << ".\n";
cin.get();
break;
case 3:
cout<<"You have selected Multiplication.\n";
cout<<"Type in the first number.\n";
cin>> x;
cin.ignore();
cout<<"Type in the second number.\n";
cin>> y;
cin.ignore();

cout<<"When you multiply " << x <<" and " << y << " you get " << x * y << ".\n";
cin.get();
break;
case 4:
cout<<"You have selected Division.\n";
cout<<"Type in the first number.\n";
cin>> x;
cin.ignore();
cout<<"Type in the second number.\n";
cin>> y;
cin.ignore();

cout<<"When you divide " << x <<" by "<< y << " you get " << x / y << ".\n";
cin.get();
break;

case 5:
cout<<"You have chosen to quit. Have a nice day and goodbye!\n";
cout<<"Hit enter one more time to exit.\n";
cin.get();
break;
default:
choice = -1;
break;
}
} while(choice == -1);

};


// These functions provide nothing that the operators do not already do
/*
int addition(int x, int y)
{
return x + y;
}

int multiply(int a, int b)
{
return a * b;
}

float division(float q, float t)
{
return q / t;
}

int subtraction(int z, int c)
{
return z - c;
}
*/





Here is a "less repetitive" version of your code, basically this makes it so that when changes are made to one part they effect the entire operation and make the code look simpler.

[source}
#include <iostream>
#include <string>

using namespace std;

void Input(int &a, int &b, string Operation);
void OutPut(int a, int b, int result, string Operation);

main()
{
int choice;
// integers on which operation will be performed
int x, y;

do
{
cout<<"Would you like me to do multiplication, division, addition, or subtraction for you?\n";
cout<<"Choose 1-5 from the following list.\n";
cout<<"1. Addition\n";
cout<<"2. Subtraction\n";
cout<<"3. Multiplication\n";
cout<<"4. Division\n";
cout<<"5. Exit\n";
cin>> choice;

switch (choice)
{
case 1:
Input(x, y, "Addition");
OutPut(x, y, x + y, "Add");

break;
case 2:
Input(x, y, "Subtraction");
OutPut(x, y, x - y, "Subtract");

break;
case 3:
Input(x, y, "Division");
OutPut(x, y, x / y, "Divide");

break;
case 4:
Input(x, y, "Multiplication");
OutPut(x, y, x / y, "Multiply");

case 5:
cout<<"You have chosen to quit. Have a nice day and goodbye!\n";
cout<<"Hit enter one more time to exit.\n";
cin.get();
break;
default:
choice = -1;
break;
}
} while(choice == -1);

};

void Input(int &a, int &b, string Operation)
{
cout << "You have selected " + Operation << endl;
cout << "Type in the first number.\n";
cin >> a;
cin.ignore();
cout << "Type in the second number.\n";
cin >> b;
cin.ignore();
}

void OutPut(int x, int y, int result, string Operation)
{
cout<<"When you " + Operation + " " << x <<" and "<< y << " you get " << result << ".\n";
cin.get();
}




there is probably even more things you could do with that code but I'll leave it up to you. The solution here to your actual question is that if "choice" is none of the "case values" than control falls to the default case in which choice is set to -1 which causes the loop to repeat. Always remember the default cause is you "catch all" if something goes wrong

Hope I Helped [grin]

Share this post


Link to post
Share on other sites
cmalune    122
And for even shorter code:

(defconstant *menu-output*
"Would you like me to do multiplication, division, addition, or subtraction for you?~%
Choose 1-5 from the following list.~%
1. Addition~%
2. Subtraction~%
3. Multiplication~%
4. Division~%
5. Exit~%")

(defun prompt-user (name operator)
(format t "~%You have selected ~a~%
Type in number 1: ~%" name)
(let ((number-1 (parse-integer (read-line))))
(format t "Type in number 2: ~%")
(let* ((number-2 (parse-integer (read-line)))
(result (funcall operator number-1 number-2)))
(format t "Result: ~a~%" result))))

(defun prompt-input (old-input)
(if (= old-input 5)
'program-terminated
(progn
(format t *menu-output*)
(let ((new-input (parse-integer (read-line))))
(case new-input
(1 (prompt-user 'addition #'+))
(2 (prompt-user 'subtraction #'-))
(3 (prompt-user 'multiplication #'*))
(4 (prompt-user 'division #'/))
(5 t))
(prompt-input new-input)))))

(prompt-input 0)

Share this post


Link to post
Share on other sites
Palidine    1315
Quote:
Original post by cmalune
And for even shorter code:


cept he's asking for help learning C++, not Scheme/Lisp so while *neat* it's not really a helpful reply

-me

Share this post


Link to post
Share on other sites
HeyHoHey    122
thanks raptor i have a question or two about your code. i understand most of it though.

#include <string>

using namespace std;

void Input(int &a, int &b, string Operation);
void OutPut(int a, int b, int result, string Operation);

main()
{
int choice;
// integers on which operation will be performed
int x, y;

do
{
cout<<"Would you like me to do multiplication, division, addition, or subtraction for you?\n";
cout<<"Choose 1-5 from the following list.\n";
cout<<"1. Addition\n";
cout<<"2. Subtraction\n";
cout<<"3. Multiplication\n";
cout<<"4. Division\n";
cout<<"5. Exit\n";
cin>> choice;

switch (choice)
{
case 1:
Input(x, y, "Addition");
OutPut(x, y, x + y, "Add");

break;
case 2:
Input(x, y, "Subtraction");
OutPut(x, y, x - y, "Subtract");

break;
case 3:
Input(x, y, "Division");
OutPut(x, y, x / y, "Divide");

break;
case 4:
Input(x, y, "Multiplication");
OutPut(x, y, x / y, "Multiply");

case 5:
cout<<"You have chosen to quit. Have a nice day and goodbye!\n";
cout<<"Hit enter one more time to exit.\n";
cin.get();
break;
default:
choice = -1;
break;
}
} while(choice == -1);

};

void Input(int &a, int &b, string Operation)
{
cout << "You have selected " + Operation << endl;
cout << "Type in the first number.\n";
cin >> a;
cin.ignore();
cout << "Type in the second number.\n";
cin >> b;
cin.ignore();
}

void OutPut(int x, int y, int result, string Operation)
{
cout<<"When you " + Operation + " " << x <<" and "<< y << " you get " << result << ".\n";
cin.get();
}





there is probably even more things you could do with that code but I'll leave it up to you. The solution here to your actual question is that if "choice" is none of the "case values" than control falls to the default case in which choice is set to -1 which causes the loop to repeat. Always remember the default cause is you "catch all" if something goes wrong



k i get the default, i like the way you did it a lot more. i never thought of having default make the variable = to -1 then have the condition for the while check for -1. now onto my question so here is the function

void OutPut(int x, int y, int result, string Operation)
{
cout<<"When you " + Operation + " " << x <<" and "<< y << " you get " << result << ".\n";
cin.get();
}

is string a type of variable and operation is the name of the variable? where did you ever introduce it? also i never saw result define anywhere.

then back to where you called the function.

case 1:
Input(x, y, "Addition");
OutPut(x, y, x + y, "Add");
so "Add" is obviously the string and x+y is the result. is that a way you can define variables through calling functions? i thought the variables in the parethesis were what your sending to the function. i assumed you needed them defined... maybe not though. also back to string so is sting similar to int float and char? could i do string = "Hello" ? or do i need to do stringA = "Hello";

thanks
hey ho
also thank you for showing me how to shorten my code its interesting to see how you did it. doing things in that way never crossed my mind before.

Share this post


Link to post
Share on other sites
nobodynews    3126
where is result defined? It's defined on the line:

void OutPut(int x, int y, int result, string Operation)

int result says that an integer is being copied into the function and this copy will be called result. I can only guess that you were confused because the code that called it:

OutPut(x, y, x / y, "Multiply");

doesn't mention result or Operation. Here's what happens:

The compiler looks at each argument, evaluates it, and then copies that value into the function definition above(void Output(int x, int y, int result, string Operation) ) in the correct spot. So the compiler looks at the argument x and evaluates it and takes the value(whatever the user entered) and copies that value to the NEW variable x. This x is not the same as the other, it's a copy. Then it evaluates y and copies that value into the NEW variable y. Next it evaluates x/y and copies That value and stores it into the NEW variable result. Finally the code takes "Multiply", converts it to a string, and copies that string to Operation.

Hope that helps.

Share this post


Link to post
Share on other sites
raptorstrike    181
O yes I forgot to mention, std::string s are most definitely your friend, they are part of the standard c++ library (hence the std namespace). To use the string type all you have to do is add

#include <string>

they are basically arrays of characters done right, you can add strings, search strings, modify strings and convert them to constant characters when a function calls for it. Here is a nice reference http://www.cppreference.com/cppstring/index.html
thats a nice helpful list of all of the functions that are available to strings but by no means do you have to know all of that just to use them, feel free to just use them to store strings and use the other stuff as you need it

strings are assigned just like integers and can be reassigned in much the same way

string Store_Characters = "characters to store.";
string Some_Other_Char = "another set of characters";
Store_Characters = "something else to store, how fun";

As for the function parameters, I purposely did this because it seemed, based on your code, that you thought that function parameters had to reference some predefined variables however, the name of the formal function parameter has nothing to do with outside variables. For example


int Add(int X, int Y)
{
return X + Y;
}


is the EXACT same thing as


void Another_Add(int A, int B) // A and B are simply function aliases for whatever
{ //value happens to be passed into this function
return A + B;
}


later in code these functions will produce the same result so


int Q, W;
Q = 5;
W = 3;
cout << Add(Q,W); //these will print the same values
cout << Another_Add(Q,W);


In fact even if the variables with the same name are defined this still wont effect the function at all.

so in this example


int A, B, X, Y;
int Q, W;
A = B = X = Y = 0;
Q = 5;
W = 3;
cout << Add(Q,W); //these values will be the same as in the above
cout << Another_Add(Q,W); //example because the assignment of A,B,X and Y
//have no effect on the functions even though their
//formal parameters are named int A, int B and
//int X, int Y respectively


In fact in a function prototype there doesn't even need to *be* a name next to the types


void Add(int, int);

//later

int Add(int SOMETHING_RANDOM_AND_UNDEFINED, int Y)
{
return SOMETHING_RANDOM_AND_UNDEFINED + Y;
}


this is perfectly fine, although many times you will want to name the parameters in the prototype, especially in classes where the header prototypes can be used in auto completion IDE features.

Hopefully this cleared up some misconceptions that the formal parameters of a function need to be named after some already existing variable. C++ can be a tough language to learn but stick with it, we are glad to have you on board and there will always be people here to help you out (and a whole lot of good books too).

NOTE: this is just an example, it still holds that Add(int K, int L) is not a useful function, just add them with the operator

Share this post


Link to post
Share on other sites
HeyHoHey    122
yup a lot of things are now cleared up in my head. thank you for the responses. right now im practicing those things to try and get comftorable with them. without all of your help i wouldnt be able to learn c++ :P.

thanks again for all the replys and info

heyho

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this