Issue with CODE !!

R

Rohan Seth

Hi
I am teaching myself C++ and have written a small program. Its a
menu driven program that accepts two numbers and gives U the option of
either adding,subtracting or multiplying the two numbers. The problem
that I am facing is that when I RUN the program the result are as follows:-


Enter M for MENU or Q to QUIT
M //I enter m or M as the menu wishes
Enter Two Numbers

The FIRST Number is: 3 // enter the number 3

The SECOND Number is: 2 // enter a number 2
Enter Your Choice:
1: ADD
2: SUBTRACT
3: MULTIPLY
You Entered: 1 // choose the option to add

The answer is: 5 // it gives me the answer
--->Enter M for MENU or Q to QUIT // this is the PROBLEM (prints twice)
PLS re-enter your data
--->Enter M for MENU or Q to QUIT

"Enter M for MENU or Q to QUIT" this statement is printed twice after
each operation. It should be printed only once. I have a feeling its
taking in some junk value....I have scratched my head for sometime with
this and now I turn for HELP .... so if any one out there has the time
and effort to compile the code(which I have pasted below) and find a
solution ...... UR EFFORTS WILL BE DEEPLY APPRECIATED !!!!

Cheers
Rohan

--------SOURCE CODE----------

#include <iostream>
#include <stdlib>
#include <string>

#define TRUE 1;
#define FALSE 0;

/*:: scope resolution operator informing the compiler that cout,endl and
cin are global identifiers and not local identifiers*/
using std::cout;
using std::cin;
using std::endl;

//Function Prototypes
int menu();
int sum(int ,int );
int subtract(int , int );
int multiply(int , int );

//function main wakes up
int main()
{
//declaring and initialising variables
int choice;
char val;
int numOne = 0;
int numTwo = 0;

//function menu wakes up and assigns the option to the variable choice
while(choice != 0)
{
//output statements
cout << "Enter M for MENU or Q to QUIT" << endl;
//accepting the value of val from the user
cin.get(val);
if(val == 'M' || val == 'm') //compares input
{
cout << "Enter Two Numbers" << endl;
cout << endl << "The FIRST Number is: ";
cin >> numOne;
cout << endl << "The SECOND Number is: ";
cin >> numTwo;

choice = menu();

switch(choice)
{
case 1: sum(numOne,numTwo);
break;
case 2: subtract(numOne,numTwo);
break;
case 3: multiply(numOne,numTwo);
break;
}
}
else if(val == 'Q' || val == 'q') //comapares input
{
cout << endl << "THX for using this program :)" << endl;
exit(0);
}
else
cout << "PLS re-enter your data" << endl;
}
return TRUE;
}

//function menu wakes up to displays the menu
int menu()
{
int option = 0;

cout << "Enter Your Choice:";
cout << endl << "1: ADD";
cout << endl << "2: SUBTRACT";
cout << endl << "3: MULTIPLY";
cout << endl << "You Entered: ";
cin >> option;

return option;
}
//function sum wakes up to add the two numbers
int sum(int nOne,int nTwo)
{
int add;

add=nOne+nTwo;
cout << endl << "The answer is: " << add << endl;

return TRUE;
}

//function subtract wakes up to subtract the two numbers
int subtract(int nOne,int nTwo)
{
int minus;

if(nOne > nTwo)
minus=nOne-nTwo;
else
{
cout << endl << "Value One is less than Value Two" << endl;
}
cout << endl << "The answer is: " << minus << endl;

return TRUE;
}

//function multiply wakes up to multiply the two numbers
int multiply(int nOne,int nTwo)
{
int product;

product=nOne*nTwo;
cout << endl << "The answer is: " << product << endl;

return TRUE;
}
 
D

David Hilsee

Rohan Seth said:
Hi
I am teaching myself C++ and have written a small program. Its a
menu driven program that accepts two numbers and gives U the option of
either adding,subtracting or multiplying the two numbers. The problem
that I am facing is that when I RUN the program the result are as follows:-


Enter M for MENU or Q to QUIT
M //I enter m or M as the menu wishes
Enter Two Numbers

The FIRST Number is: 3 // enter the number 3

The SECOND Number is: 2 // enter a number 2
Enter Your Choice:
1: ADD
2: SUBTRACT
3: MULTIPLY
You Entered: 1 // choose the option to add

The answer is: 5 // it gives me the answer
--->Enter M for MENU or Q to QUIT // this is the PROBLEM (prints twice)
PLS re-enter your data
--->Enter M for MENU or Q to QUIT

"Enter M for MENU or Q to QUIT" this statement is printed twice after
each operation. It should be printed only once. I have a feeling its
taking in some junk value....I have scratched my head for sometime with
this and now I turn for HELP .... so if any one out there has the time
and effort to compile the code(which I have pasted below) and find a
solution ...... UR EFFORTS WILL BE DEEPLY APPRECIATED !!!!

Cheers
Rohan

--------SOURCE CODE----------

#include <iostream>
#include <stdlib>

What is this header? Did you mean cstdlib or stdlib.h?
#include <string>

#define TRUE 1;
#define FALSE 0;

/*:: scope resolution operator informing the compiler that cout,endl and
cin are global identifiers and not local identifiers*/
using std::cout;
using std::cin;
using std::endl;

//Function Prototypes
int menu();
int sum(int ,int );
int subtract(int , int );
int multiply(int , int );

//function main wakes up
int main()
{
//declaring and initialising variables
int choice;

You should initialize this. From the logic below, I recommend something
that is not zero. You could also change the loop below to a do/while
instead of a while.
char val;
int numOne = 0;
int numTwo = 0;

//function menu wakes up and assigns the option to the variable choice
while(choice != 0)
{
//output statements
cout << "Enter M for MENU or Q to QUIT" << endl;
//accepting the value of val from the user
cin.get(val);
if(val == 'M' || val == 'm') //compares input
{
cout << "Enter Two Numbers" << endl;
cout << endl << "The FIRST Number is: ";
cin >> numOne;
cout << endl << "The SECOND Number is: ";
cin >> numTwo;

choice = menu();

switch(choice)
{
case 1: sum(numOne,numTwo);
break;
case 2: subtract(numOne,numTwo);
break;
case 3: multiply(numOne,numTwo);
break;
}
}
else if(val == 'Q' || val == 'q') //comapares input
{
cout << endl << "THX for using this program :)" << endl;
exit(0);
}
else
cout << "PLS re-enter your data" << endl;
}
return TRUE;
}

//function menu wakes up to displays the menu
int menu()
{
int option = 0;

cout << "Enter Your Choice:";
cout << endl << "1: ADD";
cout << endl << "2: SUBTRACT";
cout << endl << "3: MULTIPLY";
cout << endl << "You Entered: ";
cin >> option;

return option;
}
//function sum wakes up to add the two numbers
int sum(int nOne,int nTwo)
{
int add;

add=nOne+nTwo;
cout << endl << "The answer is: " << add << endl;

return TRUE;
}

//function subtract wakes up to subtract the two numbers
int subtract(int nOne,int nTwo)
{
int minus;

if(nOne > nTwo)
minus=nOne-nTwo;
else
{
cout << endl << "Value One is less than Value Two" << endl;
}
cout << endl << "The answer is: " << minus << endl;

return TRUE;
}

//function multiply wakes up to multiply the two numbers
int multiply(int nOne,int nTwo)
{
int product;

product=nOne*nTwo;
cout << endl << "The answer is: " << product << endl;

return TRUE;
}

It think that your code is reading the newline that the user entered when it
did not intend to do so. Changing

cin.get(val);

to

cin >> val;

fixed this issue on my machine. The stream operator skips whitespace, so it
ignores the newline.
 
R

Rohan Seth

THX MATE !!! worked for me too :)

David said:
What is this header? Did you mean cstdlib or stdlib.h?




You should initialize this. From the logic below, I recommend something
that is not zero. You could also change the loop below to a do/while
instead of a while.




It think that your code is reading the newline that the user entered when it
did not intend to do so. Changing

cin.get(val);

to

cin >> val;

fixed this issue on my machine. The stream operator skips whitespace, so it
ignores the newline.
 
W

William Payne

Rohan Seth said:
THX MATE !!! worked for me too :)

Just one more thing:
Remove these #defines:
#define TRUE 1;
#define FALSE 0;
C++ already has keywords for boolean true and false, so using the
preprocessor in this manner is something that should be avoided. In the rest
of the code, replace all TRUE with true and all FALSE with false. And
#define is a preprocessor directive and doesn't need to end in ; but you
should really use variables if you want to define constants to use in your
code.

/ WP
 
D

David Hilsee

William Payne said:
Just one more thing:
Remove these #defines:
#define TRUE 1;
#define FALSE 0;
C++ already has keywords for boolean true and false, so using the
preprocessor in this manner is something that should be avoided. In the rest
of the code, replace all TRUE with true and all FALSE with false. And
#define is a preprocessor directive and doesn't need to end in ; but you
should really use variables if you want to define constants to use in your
code.

Well, I didn't want to go into this, but the TRUE/FALSE return codes didn't
serve any purpose at all because the functions that returned them never had
their return values checked, and they were always the same (TRUE), no matter
what happened during sum's/multiply's/subtract's execution. In that case,
it would probably be best if the functions "sum", "subtract", and "multiply"
returned void. If Rohan really wanted to use true/false return values
properly, then I agree that they should return a bool and they should return
different values depending on what happened (I/O error, successful input,
whatever).
 
J

jmh

I'm curious about the, lacking a better word, etiquette of
using return TRUE (or return true i the defines are removed)
from a function that returns an int.

I get the fact (I hope) that true and false evaluate to 1
and 0 so it's not going to generate any error but as a
general practice would a programmer or shop just wonder
why someone typed 3 or more extra key strokes and move on?

jmh
 
D

David Hilsee

jmh said:
I'm curious about the, lacking a better word, etiquette of
using return TRUE (or return true i the defines are removed)
from a function that returns an int.

I get the fact (I hope) that true and false evaluate to 1
and 0 so it's not going to generate any error but as a
general practice would a programmer or shop just wonder
why someone typed 3 or more extra key strokes and move on?

As described in the standard (4.5 ("Integral promotions")), a bool's false
becomes an int's zero, and a bool's true becomes an int's one. However, I
would not recommend using "return true"/"return false" when the return value
is declared as an int or some other type that is not bool. If the return
type for that function is bool, then feel free, because it does not imply a
mistake. If that boolean value does not match the function's return type,
then it might be confusing to the reader. If you are using an int as the
return type, then, for readability's sake, try to use an int when you use
conversions.
 
G

Greg Comeau

I'm curious about the, lacking a better word, etiquette of
using return TRUE (or return true i the defines are removed)
from a function that returns an int.

I get the fact (I hope) that true and false evaluate to 1
and 0 so it's not going to generate any error but as a
general practice would a programmer or shop just wonder
why someone typed 3 or more extra key strokes and move on?

It would indicate to me to take a second look,
if not even an indication that it's plain wrong.

That said, it seems to me that your code contains a bunch
of functions which always return TRUE (BTW, the #define's
end in a semi-colon, which probably isn't what you wanted),
and never return anything else, which probably isn't what
you wanted either.
 
J

JKop

--------SOURCE CODE----------
#include <iostream>
#include <stdlib>
#include <string>

#define TRUE 1;
#define FALSE 0;


Only use #define when C++ doesn't provide an in-built facility for what
you're trying to achieve. For example, inclusion guards in a header file.

instead, write:

int const TRUE = 1;
int const FALSE = 0;

But...

Here's the built-in C++ types:

int
unsigned
short
unsigned short
long
unsigned long
char
signed char
unsigned char
bool
wchar_t
float
double
long double

Note the type "bool". It can contain two possible values, "true" and
"false". Use the bloody thing!

/*:: scope resolution operator informing the compiler that cout,endl
and cin are global identifiers and not local identifiers*/
using std::cout;
using std::cin;
using std::endl;


Nope. That isn't what it does.

The name of the object is "std::cout". What that means is that the object
"cout" is defined inside a namespace called "std", as so:

namespace std
{
int cout;
}


When you write in your code:

using std::cout;

You're dumping it into the global namespace, which means you can simply
write:

cout

or, as it's now in the global namespace:

::cout
//Function Prototypes
int menu();
int sum(int ,int );
int subtract(int , int );
int multiply(int , int );

//function main wakes up
int main()
{
//declaring and initialising variables
int choice;
char val;
int numOne = 0;
int numTwo = 0;

//function menu wakes up and assigns the option to the variable
choice while(choice != 0)


When you define a variable like so:

int k;


"k" contains no particular value. You define "choice" like this, and then
you check it's value, which may be 0, or could be 76, or could be 25, or 32.

{
//output statements
cout << "Enter M for MENU or Q to QUIT" << endl;


Write '\n' instead of endl:

std::cout << '\n';

//accepting the value of val from the user
cin.get(val);
if(val == 'M' || val == 'm') //compares input
{
cout << "Enter Two Numbers" << endl;
cout << endl << "The FIRST Number is: ";
cin >> numOne;
cout << endl << "The SECOND Number is: ";
cin >> numTwo;

choice = menu();

switch(choice)
{
case 1: sum(numOne,numTwo);
break;
case 2: subtract(numOne,numTwo);
break;
case 3: multiply(numOne,numTwo);
break;
}
}
else if(val == 'Q' || val == 'q') //comapares input
{
cout << endl << "THX for using this program :)" << endl;
exit(0);
}
else
cout << "PLS re-enter your data" << endl;
}
return TRUE;
}

//function menu wakes up to displays the menu
int menu()
{
int option = 0;

cout << "Enter Your Choice:";
cout << endl << "1: ADD";
cout << endl << "2: SUBTRACT";
cout << endl << "3: MULTIPLY";
cout << endl << "You Entered: ";
cin >> option;

return option;
}
//function sum wakes up to add the two numbers
int sum(int nOne,int nTwo)
{
int add;

add=nOne+nTwo;


There's no need for a new variable. Just use the original:

nOne+=2;

std::cout << "\nThe answer is: " << nOne << '\n';

Use "const" wherever you can:

int sum(int nOne, int const nTwo)
{
return nOne += 2;
}

cout << endl << "The answer is: " << add << endl;

return TRUE;
}

//function subtract wakes up to subtract the two numbers
int subtract(int nOne,int nTwo)
{
int minus;


Again, no new variable is needed.

If you want to call "nOne" by a different name, then:

int& minus = nOne;

if(nOne > nTwo)
minus=nOne-nTwo;
else
{
cout << endl << "Value One is less than Value Two" << endl;
}
cout << endl << "The answer is: " << minus << endl;

return TRUE;
}

//function multiply wakes up to multiply the two numbers
int multiply(int nOne,int nTwo)
{
int product;

product=nOne*nTwo;
cout << endl << "The answer is: " << product << endl;

return TRUE;
}


-JKop
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,904
Latest member
HealthyVisionsCBDPrice

Latest Threads

Top