1. We have moved from vBulletin to XenForo and you are viewing the site in the middle of the move. Though the functional aspect of everything is working fine, we are still working on other changes including the new design on Xenforo.
    Dismiss Notice

Another Question, And apology included...

Discussion in 'C++' started by danielakkerman, Dec 27, 2006.

  1. danielakkerman

    danielakkerman New Member

    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
     
  2. DaWei

    DaWei New Member

    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: Dec 27, 2006
  3. pradeep

    pradeep 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.
     
  4. shabbir

    shabbir Administrator Staff Member

    The link explains the problem but the summary is you should be providing the address of the variable and not just the variable name.
     
  5. danielakkerman

    danielakkerman New Member

    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
     
  6. shabbir

    shabbir Administrator Staff Member

    Try reading the content of the post specified by DaWei
     
  7. DaWei

    DaWei New Member

    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: Dec 28, 2006
  8. danielakkerman

    danielakkerman New Member

    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
     
  9. DaWei

    DaWei New Member

    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;
    }
    
     
  10. danielakkerman

    danielakkerman New Member

    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
     

Share This Page