Another Question, And apology included...

danielakkerman's Avatar, Join Date: Dec 2006
Light Poster
Hello,
Let me start by thanking all of those who had helped me with "When null when not..." , that was absolutely wonderful, solved all issues.
I am sorry I wasn't able to post any thanking thread; I had an internet breakdown, so that kept me away from the web for a while.
So again, to all who were involved, thanks, and thanks again.
Now for the problem itself:
Code:
int Menu(){
	int i;
    char *In = (char *)malloc(256);

	cout<< "Option 1:" << "\n";
	cout<< "Option 2:" << "\n";
	cout<< "Option 3:" << "\n";
	cout<< "ATTENTION: Please type the number of the selection only!" << "\n";
	scanf("%s", In);
	i = atoi(In);
	if(i == 0){
		system("CLS");
		Menu();
	}
	return i;
}
The problem is that when a string is typed, but afterwards A VALID int is inserted, it disregards that int, and once again performs the recursion ("Menu();").
So basically, what I am saying is, that whenever a string is typed, it is all good, it repeats the function, and all's well.
But after that string, if you'll try to type an integer, it will ignore that fact (that the new input is an Integer), and will repeat the function again, and that's not good...
Help?
Thanks
Very sorry for my rudeness again,
Thanks for the patience and generosity, (hopefully also forgiveness),
Daniel
DaWei's Avatar, Join Date: Dec 2006
Team Leader
See this thread. If you don't get the point, post back. Personally, I would not use atoi (), either. I also see no valid reason to use menu () recursively.

Last edited by DaWei; 28Dec2006 at 03:42..
pradeep's Avatar, Join Date: Apr 2005
Team Leader
This I think will consume a lot of stack space if it recurses a lot of times. I guess loops are a more efficient way of implementing this.
shabbir's Avatar, Join Date: Jul 2004
Go4Expert Founder
Quote:
Originally Posted by DaWei
See this thread. If you don't get the point, post back. Personally, I would not use atoi (), either. I also see no valid reason to use menu () recursively.
The link explains the problem but the summary is you should be providing the address of the variable and not just the variable name.
danielakkerman's Avatar, Join Date: Dec 2006
Light Poster
Hi,
Thanks for your help, but I still don't get it.
The recursion should only take place, btw, if what the user had typed was a string...
Would like to hear more...
Daniel
shabbir's Avatar, Join Date: Jul 2004
Go4Expert Founder
Quote:
Originally Posted by danielakkerman
Hi,
Thanks for your help, but I still don't get it.
The recursion should only take place, btw, if what the user had typed was a string...
Would like to hear more...
Daniel
Try reading the content of the post specified by DaWei
DaWei's Avatar, Join Date: Dec 2006
Team Leader
You have a number of serious issues. In the first place, using menu recursively is not a good idea. It is not showing that you're a slick programmer, but, rather, the opposite. Every call you make is borrowing memory from the heap. Nowhere do you show that you're freeing it. You can't be doing it elsewhere, because elsewhere has no idea how many times you've done it, or where the hell it is.

As for scanf, as mentioned in the other thread, you are not testing it to see if if succeeded in meeting your request. It does not promise to do so. It promises to do so or tell you that it failed. You aren't asking. Read about the return value of scanf and use it. When you break the stream, it stays broken until you repair it. It will zoop right through susbsequent calls. You will wonder why. Ask it. One who fails to do so is either uninformed or schlocky. You have now been informed.

I am fond of saying that the geeks who wrote these utilitarian functions knew what they were doing and provided return values (or other mechanisms for testing success) for a purpose. ATOI is an exception. ATOI will return a zero on failure. It will also return a zero if you present it with an argument that evaluates to zero. There is no way to tell the difference. Bad function, bad function (slaps it on the wrist with a ruler, just like the nuns used to do). Worse, with a bad argument, you will get undefined operation. That means the compiler is entitled to melt your system into a slag heap and eat your dog before he gets to your homework. Fortunately, most won't do that. Investigate string streams, or drop back into a C-style sscanf, or something.

Another issue is your user. You are instructing him/her to type a number. BWAHAAAAHAAAAAAAAAHAAAAA!!!! You are planning for an input of no more than 256 bytes. MUHAAAAAAHAAAAAAAAHAAAAAAA!!!1111ELEVEN. What are you going to do if you get a user who is stupid or malicious or falls asleep with his/her head on the keyboard? I know what your program is going to do. It is going to run off into the weeds and barf on its shoes.

Your code hints that you are using C++. Use C++. Read some of Bruce Eckel's stuff.

Last edited by DaWei; 28Dec2006 at 22:02..
danielakkerman's Avatar, Join Date: Dec 2006
Light Poster
Hi,
So basically, I've tried everything, and if it isn't too much trouble, can someone please rewrite my code, so that it will work.
Of course, it's not an obligation, but I am quite desperate to resolve this.
Again, I'd like to thank everyone who'd been helping me out here, thank you.
Minor Remark: I am doing my best to stick with C, because for some reason, I don't appreciate C++ too much. I feel that C's been quite neglected over time, which's a shame.
Thanks again for your help,
Thanks,
Daniel
DaWei's Avatar, Join Date: Dec 2006
Team Leader
How can you say you're sticking with C when you're using C++ streams? Everyone is entitled to their opinion, of course, but your remarks have no real technical validity. C++ will almost always result in more robust code with fewer errors, with little, if any, performance degradation. It represents a mature advance made possible by technological gains.

Here are versions for C and C++.

C++
Code:
#include <iostream>

using std::cin;
using std::cout;
using std::endl;

int menu ();

int main ()
{
	int choice = 0;
	while (choice != 3)
	{
		choice = menu ();
		cout << "Your choice was " << choice << endl;
	}
	return 0;
}

int menu ()
{
	int choice = 0;
	while (1)
	{
		cout << "Enter your selection (1, 2, 3 to exit): ";
		cin >> choice;
		if (!cin.good ()) choice = 0;
		if ((choice < 1) || (choice > 3))
			cout << "Your selection was invalid.  Try again.\n" << endl;
		else return choice;
		cin.sync ();
		cin.clear ();
	}
	return choice;
}
C
Code:
#include <stdio.h>

int menu ();

int main (int argc, char *argv [])
{
	int choice = 0;
	while (choice != 3)
	{
		choice = menu ();
		printf ("Your choice was %d\n", choice);
	}
	return 0;
}
int menu ()
{
	int choice = 0;
	while (1)
	{
		printf ("Enter your selection (1, 2, 3 to exit): ");
		if (scanf ("%d", &choice) != 1) choice = 0;
		if ((choice < 1) || (choice > 3))
			puts ("Your selection was invalid.  Try again.\n\n");
		else return choice;
		rewind (stdin);
	}
	return choice;
}
Quote:
Originally Posted by Output

E:\Documents and Settings\David\My Documents\Visual Studio 2005\Projects\Menu\de
bug>menu
Enter your selection (1, 2, 3 to exit): aaa
Your selection was invalid. Try again.

Enter your selection (1, 2, 3 to exit): bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
Your selection was invalid. Try again.

Enter your selection (1, 2, 3 to exit): 33
Your selection was invalid. Try again.

Enter your selection (1, 2, 3 to exit): 1
Your choice was 1
Enter your selection (1, 2, 3 to exit): 2
Your choice was 2
Enter your selection (1, 2, 3 to exit): 3
Your choice was 3

E:\Documents and Settings\David\My Documents\Visual Studio 2005\Projects\Menu\de
bug>
danielakkerman's Avatar, Join Date: Dec 2006
Light Poster
Hi,
It's finally working.
I can't thank you enough for your help.
Thanks everybody, and especially "DaWei", for finally "nailing" the issue.
Thanks again,
Daniel