Solved

Avoiding infinite loop

Posted on 2008-10-10
12
598 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
  • 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
 
LVL 16

Accepted Solution

by:
t0t0 earned 500 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
What Should I Do With This Threat Intelligence?

Are you wondering if you actually need threat intelligence? The answer is yes. We explain the basics for creating useful threat intelligence.

 
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

What Security Threats Are You Missing?

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

Summary: This tutorial covers some basics of pointer, pointer arithmetic and function pointer. What is a pointer: A pointer is a variable which holds an address. This address might be address of another variable/address of devices/address of fu…
When we want to run, execute or repeat a statement multiple times, a loop is necessary. This article covers the two types of loops in Python: the while loop and the for loop.
This tutorial will introduce the viewer to VisualVM for the Java platform application. This video explains an example program and covers the Overview, Monitor, and Heap Dump tabs.
The viewer will learn additional member functions of the vector class. Specifically, the capacity and swap member functions will be introduced.

760 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

Need Help in Real-Time?

Connect with top rated Experts

16 Experts available now in Live!

Get 1:1 Help Now