?
Solved

Avoiding infinite loop

Posted on 2008-10-10
12
Medium Priority
?
604 Views
Last Modified: 2011-10-19
Thanks for helping.
Borland C++4.5 Teach yourself C
Just so you know, this is not homework.  I am 67 yrs old and retired...keeping busy :0)
There are a couple of places I don't know how to prevent an infinite loop indicated by /*here*/
Also feel free to critique...Actually this is just a sample part of a much larger program...I will have more questions....lol



#include <ctype.h> // toupper
#include <conio.h> // clrscr  get char
#include <stdio.h> // std stuff
 
#define TRUE 1              //it's true
#define FALSE !TRUE         //it's not true
 
char do_again;
int menu;
char ans;
 
int main (void)
{  //  start main
  do_again=TRUE;
 
  while (do_again)
  {     //start do_again loop
  do
  {    // start menu loop
    printf("\n Enter 1. B breakfast  2. Lunch  3. Dinner\n");
    scanf("%i",&menu);
/*here*/    if ( (menu >= 4) || ( menu <=0))// || (menu == ????a letter) )  //need to prevent infinite loop when anything other than a letter is //entered.
	printf("\nNot an option, try again");
	//end menu loop
	while  ( (menu >= 4) || (menu <=0 ));// || (menu ==????a letter );
	switch (menu)
	{             //start menu switch
	  case 1:printf("\n Fix it yourself!\n");
	       break;
	  case 2:printf("\n You just ate!\n");
		 break;
case 3:printf("\nIf you worked you could take us out to eat!\n");
		break;
		}   //end menu switch
		/*find out if user wants to do some more of this  */
printf("\nStill Hungry? N for NO,any key for yes\n");
		 ans=toupper(getch());
/*here*/		 if(ans=='N')  //this can get into an unwanted loop also
			{
		  //end if
			do_again=!TRUE;
			printf("\n\nAdios");
			}
 
  }	 // end do_again loop
		 return 0;
}      //end main

Open in new window

0
Comment
Question by:FrawgLips
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 5
  • 4
  • 2
  • +1
12 Comments
 
LVL 45

Expert Comment

by:sunnycoder
ID: 22688769
/*here*/    if ( (menu >= 4) || ( menu <=0))// || (menu == ????a letter) )  //need to prevent infinite loop when anything other than a letter is //entered.
        printf("\nNot an option, try again");
        //end menu loop   --->>>continue statement would take you to the beginning of the loop
        while  ( (menu >= 4) || (menu <=0 ));// || (menu ==????a letter );   --->> remove the ; ... this ; after while makes it an infinite loop


0
 
LVL 53

Expert Comment

by:Infinity08
ID: 22688772
Input using scanf is always tricky, since any unread (invalid) characters stay on the stream, and are processed again and again and ... creating an infinite loop.

You should use a more robust way of inputting data - ie. do it line-based by using fgets :

        http://www.cplusplus.com/reference/clibrary/cstdio/fgets.html

Read one line of data into a string, and then get the values out of that string (using sscanf for example which is very similar to scanf in usage)

        http://www.cplusplus.com/reference/clibrary/cstdio/sscanf.html

sscanf can catch invalid data (its return value indicates how many values were successfully read, so if you try to read one integer value, then it should return 1 - if it doesn't, then an error occurred, and the data should not be used).
In case of invalid data, you can simply ask the user to enter the input again. No garbage will be left on the stream since fgets already got that out.
0
 
LVL 45

Expert Comment

by:sunnycoder
ID: 22688816
If you keep the code indentation right, you would increase the readability as well as spot lot of errors.
The outer while loop is not required. What you need is

do
{
     print menu
     read choice
     process choice
     ask if user wants to continue
} while (user said yes);  --> ; at the end of while of a do while loop is required ... at the end of a simple while makes it an infinite loop.
0
What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

 
LVL 16

Accepted Solution

by:
t0t0 earned 2000 total points
ID: 22691472
frawglip.....

It's been many years since i've looked beneath the bonnet of C and C++. Please take a glance at the simple menu i've written. Hopefully it will help you with your project.

It's a very simple menu which you can customise for your own use. It also answers your problem of loops. Notice by splitting it up into smaller functions I was able to add user input validation which is hidden from the main function.

notice how uncluttered the code looks. this is important in C as is lots and lots of comments.

the screen output is simplified for illustrations only....


//==============================================================================
// simple menu
//
// written by paul tomasi - 10 October 2008
//==============================================================================
#include<stdio.h>
#include<ctype.h>

void display_menu(void);                 // Display menu
char get_option(void);                      // Get user input 1-3
char have_another_go(void);          // Get user input Y/N

void option_one(void);                     // Function 1, 2 & 3
void option_two(void);
void option_three(void);


//------------------------------------
// DISPLAY MENU
//------------------------------------
void display_menu(){
  printf("\n\n1 - Option One\n\n");    // Display available options
  printf("2 - Option Two\n\n");
  printf("3 - Option Three\n\n");
}


//------------------------------------
// GET OPTION
//------------------------------------
char get_option(){
  char ch = ' ';                                  // Reset character
 
  printf("Select option 1-3: ");          // Display prompt
 
  while(ch < '1' || ch > '3'){               // Loop until character is 1, 2 or 3
    ch = getchar();                            // Get keyboard character
  }
 
  return ch;                                      // Pass character back
}
 

//------------------------------------
// HAVE ANOTHER GO
//------------------------------------
char have_another_go(){
  char ch = ' ';                                  // Reset character
 
  printf("\n\nContinue (Y/N): ");       // Display prompt
 
  while(ch != 'Y' && ch != 'N'){         // Loop until character is Y or N
    ch = toupper(getchar());            // Get K/B char and convert to uppercase
  }

  return ch;                                     // Pass character back
}


//------------------------------------
// OPTION 1
//------------------------------------
void option_one(void){
  printf("\n\nYou chose option 1");   // Do something
}


//------------------------------------
// OPTION 2
//------------------------------------
void option_two(void){
  printf("\n\nYou chose option 2");   // Do something
}


//------------------------------------
// OPTION 3
//------------------------------------
void option_three(void){
  printf("\n\nYou chose option 3");   // Do something
}


//------------------------------------
// MAIN FUNCTION
//------------------------------------
int main(){
  char option = 'Y';                         // Reset option
   
  while(option == 'Y'){                    // Loop until option is N
    display_menu();                         // Display menu options
    option = get_option();                 // Get option from user

    switch(option){                          // Decide what option to execute
      case '1': option_one();             // Do option 1
                break;
      case '2': option_two();            // Do option 2
                break;
      case '3': option_three();          // Do option 3
                break;
    }
     
    option = have_another_go();     // Does the user want to go again?
  }    
}

//==============================================================================
// simple menu
//
// written by paul tomasi - 10 October 2008
//==============================================================================
#include<stdio.h>
#include<ctype.h>
 
void display_menu(void);              // Display menu
char get_option(void);                // Get user input 1-3
char have_another_go(void);           // Get user input Y/N
 
void option_one(void);                // Function 1, 2 & 3
void option_two(void);
void option_three(void);
 
 
//------------------------------------
// DISPLAY MENU
//------------------------------------
void display_menu(){
  printf("\n\n1 - Option One\n\n");   // Display available options
  printf("2 - Option Two\n\n");
  printf("3 - Option Three\n\n");
}
 
 
//------------------------------------
// GET OPTION
//------------------------------------
char get_option(){
  char ch = ' ';                      // Reset character
  
  printf("Select option 1-3: ");      // Display prompt
  
  while(ch < '1' || ch > '3'){        // Loop until character is 1, 2 or 3
    ch = getchar();                   // Get keyboard character
  }
  
  return ch;                          // Pass character back
}
  
 
//------------------------------------
// HAVE ANOTHER GO
//------------------------------------
char have_another_go(){
  char ch = ' ';                      // Reset character
  
  printf("\n\nContinue (Y/N): ");     // Display prompt
  
  while(ch != 'Y' && ch != 'N'){      // Loop until character is Y or N
    ch = toupper(getchar());          // Get K/B char and convert to uppercase
  }
 
  return ch;                          // Pass character back
}
 
 
//------------------------------------
// OPTION 1
//------------------------------------
void option_one(void){
  printf("\n\nYou chose option 1");   // Do something
}
 
 
//------------------------------------
// OPTION 2
//------------------------------------
void option_two(void){
  printf("\n\nYou chose option 2");   // Do something
}
 
 
//------------------------------------
// OPTION 3
//------------------------------------
void option_three(void){
  printf("\n\nYou chose option 3");   // Do something
}
 
 
//------------------------------------
// MAIN FUNCTION
//------------------------------------
int main(){
  char option = 'Y';                  // Reset option
    
  while(option == 'Y'){               // Loop until option is N
    display_menu();                   // Display menu options
    option = get_option();            // Get option from user
 
    switch(option){                   // Decide what option to execute
      case '1': option_one();         // Do option 1
                break;
      case '2': option_two();         // Do option 2
                break;
      case '3': option_three();       // Do option 3
                break;
    }
     
    option = have_another_go();       // Does the user want to go again?
  }     
}

Open in new window

simpmenu.txt
0
 

Author Comment

by:FrawgLips
ID: 22692084
t0t0
thanks,  I messed up by editing after attaching...it looks more orderly on my machine....
I got a good laugh out of your comment..."It's a very simple menu...there's enough  there to keep me busy for quite a while  thanks.  so far so good, but, if a  letter instead of 1 2 or 3  is entered, I would need to say "not an option" or some such thing...I don't see where or how  to put that...I tried a  printf('\nOOps!!"):
in the GET OPTION area but no go...

0
 
LVL 53

Expert Comment

by:Infinity08
ID: 22693163
The accepted code will not solve all input problems, since getchar still leaves the rest of the line (at least the newline) on the input stream. The next time input is needed, it will first have to wade through this garbage, and will usually not be able to cope with that correctly.

I would really suggest to consider a line-based input system using fgets like I suggested, as well as fixing the problems pointed out by sunnycoder.
If your intention is to learn, then you'll learn a lot more by fixing your own code and getting it to work, than by throwing away your code and using someone else's. Learning from your own mistakes is one of the best ways to learn, especially if you discover the mistakes and their solution yourself.
0
 
LVL 16

Expert Comment

by:t0t0
ID: 22695277
infinity08

"The accepted code will not solve all input problems"

Have you copied and pasted the code into your IDE and tested it for acceptance? Be aware that although I addressed issues of validation, the original requirement was to provide a structure whereby the looping, in this case two loops, are coded into a working model.

One does not always learn by their own mistakes, Program design and coding, are creative processes where many solutions may exist. Some solutions more elegant than others. C can be a very cryptic language even at the best of times and in this case I have attempted to show clarity as well as good program design. One can learn from others, and I believe Frawglips' aim is to arrive at a solution as rapidly as possible so that he can concentrate on the overall project in hand - and this was apparant by the number of points Frawglips allocated to his question.

I have divided the solution into manageable functions isolating the the nuts and bolts of the menu structure so that should Frawglips want to change a particular part of the code then he can do without effecting other parts.

I have also incorporated local variables within functions whose values are passed back in the calling expression. I believe Frawglips will learn by customising my design to suit his own application.

With regard to your suggestion to use a line-based input, I personally always write my own input functions from the ground up with inbuilt validation and then overloading the function for different applications. Needless to say, these input functions are extremely robust and behave exactly as intended. But then we're detracting ....

Sunnycoder makes a valid point about indentation. This should be adjustable in the IDE options to 2, 3 or even 4 character spaces. I personally prefer 3 although I have used only 2 on this occasion.


Frawglips

If you are still stuck on the "Oops..." thing then please post back. Notice how the prompt itself informs the user of the input required. If you really feel the need to bambard the user with a popup message saying he hit the wrong key then you can insert this inside the WHILE loop in the get_option() and have_another_go() functions directly after the 'ch=' assignment. This could consist of a simple IF conditional statement however, I would advise you rather than popping up a message to which the user must then press another key or enter another character before proceding, perhaps you would consider a simple beep alerting the user so that it does not impede his normal rhythm of operation.

Thank you  for the points. It was a pleasure.

The revised function below will sound a beep warning the user of errors although you will need to write a beep() funtion to achieve this. This might be platform specific.
//------------------------------------
// GET OPTION
//------------------------------------
char get_option(){
  char ch = ' ';
 
  printf("Select option 1-3: ");
  
  while(ch < '1' || ch > '3'){
    ch = getchar();
    
    if(ch < '1' || ch > '3')    // Same as loop condition    
      beep();                   // Can be replaced with a message
  }
  
  return ch;
}

Open in new window

0
 
LVL 53

Expert Comment

by:Infinity08
ID: 22695429
>> Have you copied and pasted the code into your IDE and tested it for acceptance?

I don't need to, since I know perfectly well how the code will behave ;) getchar reads one character, but on a lot of systems, input IS line based - ie. a character input by the user will only be available to the code when the user presses the ENTER key.
In other words, on a lot of systems, getchar will get that character, but it will leave the newline (and any other garbage) on the stream. The next time something needs to be read from input, this newline will still be there ... and might or might not interfere with the correct handling of input.
Sure, your framework works on its own, since you discard any characters that are not valid menu entries (so that includes the newline). But a menu on its own isn't very useful, and menu options generally have code behind them that needs to be executed, and this code could very well also require input from the user. Which brings me back to the point I made :

"The accepted code will not solve all input problems, since getchar still leaves the rest of the line (at least the newline) on the input stream. The next time input is needed, it will first have to wade through this garbage, and will usually not be able to cope with that correctly."


The reason I highly recommend a line-based input method is because it avoids all these problems in a very easy way, and makes the code very robust.
0
 
LVL 16

Expert Comment

by:t0t0
ID: 22696635
Infinity08

Thank you for your late posting on this matter. You make a number of valid points. You are without doubt and accomplished programmer and valued contributor in many areas of EE. I acknowledge your expertise in this field and respect your points of views.

I take on board your comments regarding the use of getchar() however, it's use on this occasion serves primarily as an arbitrary method to input characters from the user.

What I tried not to detract from is Frawglips' requirement specification - simply put, he admits not knowing how to prevent infinite loops. I trust you'll agree that in doing so, the principle areas of concern are:

(1) the declaration and intitialisation ch and option whose data is key to the logic and design of the looping structures,

(2) the loops themselves along with their conditional statements,

(3) the acquisition of user input (not the method),

(4) how the user input is validated and passed back to the main program and

(5) how the data is implemented in the menu selection process.

I agree getchar() may not be the best choice here however, as part of stdin it is guaranteed to work on all compilers. That said, please feel free to provide a working example of a line-based input routine - perhaps Frawglips may substitute it into his get_option() function.
0
 
LVL 53

Expert Comment

by:Infinity08
ID: 22696976
No big changes are needed. Simply replace the scanf's that he has with sscanf's, and read a line of input into a line buffer using fgets. That's all.


FrawgLips' original code had infinite loops partly because the manner of input left garbage on the stream, which the rest of the code then tripped over. Your framework does not solve that in its own, so there's a good chance that using it would leave FrawgLips with getting the same (similar) infinite loops - thus not solving his original problem.


In any case, I do not contest that you get the points - you earned them (and we need all the experts we can get here - new blood is always welcome). I simply want to make sure that FrawgLips' problem is completely solved :)
0
 
LVL 16

Expert Comment

by:t0t0
ID: 22699053
Thank you infintiy08

I believe Frawglips has decided to take your advice afterall and learn by editing his own code.
0
 
LVL 53

Expert Comment

by:Infinity08
ID: 22700984
I hope I didn't scare you off, t0t0. Having a new expert here taking the time to provide a solution, and follow it up is much appreciated ! Welcome :)
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Navigation is an important part of web design from a usability perspective. But it is often a pain when it comes to a developer’s perspective. By navigation, it often means menuing. This is less theory and more practical of how to get a specific gro…
Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
This video teaches viewers about errors in exception handling.
The viewer will learn how to use the return statement in functions in C++. The video will also teach the user how to pass data to a function and have the function return data back for further processing.
Suggested Courses

762 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question