• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 1316
  • Last Modified:

scanf won't wait

Hi there.

I'm trying to do some error handling in my code. When the program asks for the number of resistors, I want the program to ask the question again if the user enter an invalid input (i.e. a character). So far, I have figured that if a user enters a character, scanf returns a value of "1231384169" - why is this? Also, in the if statement, if i make the program return to the beginning, scanf won't wait!! Please help. Many Thanks.

#include <stdio.h>
#include <stdlib.h>

int main()
{
      
    double * rn,rseries,reff;

    int rnumber,count;
      char start=0;
beg:
    rseries = 0;
    reff = 0;
      
      system("cls");

    printf("How many resistors do you have? ");
   
    scanf("%d",&rnumber);
 
   if(rnumber == 1231384169)
   {
         printf("\n\nInvalid Character - Program Terminated!\n\n");
         return(0);
       }
 
 
    rn = malloc(rnumber * sizeof(*rn));
      
      printf("\n");
   
      for (count=0; count < (rnumber); count++)
    {
        printf("What is the Value of R%d? ",(count + 1));
        scanf("%lf", (rn + count));
        rseries = rseries + *(rn + count);
        reff = reff + (1/ *(rn + count));
    }
   
    reff = 1/reff;
   
    printf("\n\n\nThe effective resistance of your resistors in series is %lf ohms",rseries);
    printf("\nThe effective resistance of your resistors in parallel is %lf ohms\n\n",reff);
   
      free(rn);
again:
      printf("Do you wish to start again? Press Y or N: ");

      scanf("%s",&start);

      if(start == 121 || start == 89)
      {
            goto beg;
      }
      else if(start == 78 || start == 110)
      {
            printf("\nGoodbye!\n\n");
            return(0);
      }
      else
      {
            printf("\nInvalid Character\n\n");
            goto again;
      }

   
   
}

0
jonnytabpni
Asked:
jonnytabpni
  • 4
  • 4
  • 3
  • +3
1 Solution
 
itsmeandnobodyelseCommented:
You would need to evaluate the return value of scanf. It returns 1 if it successfully  converted a number and 0 in the case a wrong conversion was made.

Note, you shouldn't use goto in C programming. Use a while loop instead and break it if the user has inputted valid data.

Regards, Alex
0
 
jonnytabpniAuthor Commented:
sorry i'm a bit confused.

please bear in mind i'm VERY new to this.

after a character was entered, rnumber was equal to 1231384169 but i'm not sure why

cheers
0
 
itsmeandnobodyelseCommented:
>>>> rnumber was equal to 1231384169
cause you didn't initialize it. Conversion to an integer fails if the user enters a char. Then scanf returns 0 what means that it scanned 0 items from the input stream. You need to check for the return value:

...
int rnumber = 0, count = 0;  /* always intialize variables */

while (true)    /* make an infinite loop while getting a valid input  */
{
   ...
   if (scanf("%d", &rnumber) == 0)  /* input is not a number */
   {
          printf("wrong number\n\n");
          continue;  /* jump at end of while loop */
   }
   /* put here the check of the rnumber */
   ...

    break;  /* break the while(true) if all is ok */
}
 
Regards, Alex
0
Concerto's Cloud Advisory Services

Want to avoid the missteps to gaining all the benefits of the cloud? Learn more about the different assessment options from our Cloud Advisory team.

 
waysideCommented:
> Also, in the if statement, if i make the program return to the beginning, scanf won't wait!!

This is a problem with scanf that I have run into in the past as well. It somehow gets confused when the input is not a number.

The solution I use is to read the input as a string and then convert it to a number:

char buf[100];

while (true)
{
  printf("How many resistors do you have? ");
  scanf("%s", buf);
  if (sscanf(buf, "%d", &rnumber) != 1) {
     // bad input
    printf("Please input a number\n");
    continue;
  }

  break; // we got a number

}
0
 
Infinity08Commented:
>> It somehow gets confused when the input is not a number.

It doesn't get confused. You're telling it to read a number, but you're not passing it a number, so it can't read it. Sounds entirely normal to me. And that's what the return value of scanf is for (as Alex already explained) : to tell you how many values it successfully read.
If you don't check the return value, then you can't blame scanf if your code does unexpected things :)
0
 
jonnytabpniAuthor Commented:
the thing is Infinity08, even when you check the return value of scanf, it still won't wait second time round
0
 
Infinity08Commented:
>> the thing is Infinity08, even when you check the return value of scanf, it still won't wait second time round

Well, you don't just have to check the return value - you also have to do womething about the problem and put the stream in a good state to continue reading from it. In other words, you read whatever is left on the stream before continuing (you discard the wrong input). Something like :

    int value = 0;

    printf("Give a value ... ");
    while (!scanf("%d", &value)) {
      char discard[100];
      fgets(discard, 100, stdin);
      printf("Try again ... ");
    }

    printf("Value read : %d\n", value);

Note that I'm assuming a maximum of 99 characters to be discarded ...


A few more comments about your code :

1) Always initialize variables when you declare them - it'll save you a lot of trouble. You didn't do that for these :

            double * rn,rseries,reff;
            int rnumber,count;


2) Don't use goto - instead make use of loops and other control structures :

        http://www.cplusplus.com/doc/tutorial/control.html


3) A check like this won't help you :

        if(rnumber == 1231384169)
       {
           printf("\n\nInvalid Character - Program Terminated!\n\n");
           return(0);
       }

    because the value 1231384169 is arbitrary and might be different the next time.


4) This is very dangerous :

            rn = malloc(rnumber * sizeof(*rn));

    you're dereferencing rn before it has been allocated (*rn). Use this instead :

            rn = (double*) malloc(rnumber * sizeof(double));


5) instead of *(rn + count) you can use rn[count] ... It's easier to read.


6) ref = ref + ...;  can be shorter by using ref += ...;


7) if you perform a division, like in 1/ *(rn + count) and 1 / reff, then make sure that the value you're dividing by is NOT 0. Otherwise you'll get a nice crash (division by 0).


8) start is a char, so this line :

        scanf("%s",&start);

    should read a char (%c) instead of a string (%s) :

        scanf("%c", &start);

    Better would be to read a string, and check the first character.


9) avoid magic numbers (numbers that are hard-coded, and difficult to know what they mean) like in :

        if(start == 121 || start == 89)

    You can use the corresponding character literals instead :

        if (start == 'y' || start == 'Y')

    It's a lot easier to read and understand.
0
 
jonnytabpniAuthor Commented:
thank you very much for the above tips :)
0
 
jonnytabpniAuthor Commented:
oh btw, with point 8 on that list, the reason why i didnt use %c was that scanf won't wait for some strange reason!
0
 
itsmeandnobodyelseCommented:
Is it a C++ prog or a C prog? Or better asked: Is it a .cpp or a .c file?

If it is c++ you might consider using

   char c;
   cin >> c;

instead of scanf. It is safer as the compiler checks the operands what isn't the case with scanf (and printf).

0
 
Infinity08Commented:
>> oh btw, with point 8 on that list, the reason why i didnt use %c was that scanf won't wait for some strange reason!

Then you need to make star a string (char array) instead of a single char.
0
 
peetmCommented:
I have read all the stuff here, but on a quick glance, wasn't the OP having trouble getting scanf to wait the second, subsequent time?

Although it's not standard C, use fflush(stdin); after each call to scanf();
0
 
Infinity08Commented:
>> wasn't the OP having trouble getting scanf to wait the second, subsequent time?

That's what we're dealing with atm ;) First we had to fix the first problem of the wrong input, which caused the "no scanf wait" problem.
0
 
peetmCommented:
Or, a routine like this whenever you want to eat what's in the 'input pipe'.

void flushStdin(void)
{
   int ch;

   do
   {
   
      ch = getc(stdin);
     
   } while (('\n' != ch)  && (EOF != ch));

}
0
 
dgraingerCommented:
jonnytabpni,

Using scanf is usually problematic for most junior to intermediate programmers.

As people have pointed out, you need to make sure the scanf was successful. If it was not then nothing was written to rnumber, i.e. rnumber will contain random data.

I usually recommend using fgets to read the user input into a string. Then using the string functions to manipulate it. Use:

    char s[80];
    fgets(s, sizeof s, stdin);
    strtok(s, "\n"); /* just gets rid of the \n on the end of line */

Once you have something in the string you can use atoi() to convert it to an integer or better yet use strtol() to parse the string. All the strto*() functions can be checked to see if the string parsed to a number okay.

Additionally, there are a few other problems with your program:

1) The system call makes this so it only works on MS-DOS. I'd use the curses library (google for it).
2) On many systems, output is line buffered. MS-DOS is not so things like:

    printf("prompt: ");

works. To guarantee it works on all systems, add the following after all calls not ending "\n":

    fflush(stdout);

3) Your if(rnumber) statement should really be checking the return code of scanf or better still avoid scanf.
4) You should check that the malloc worked okay. If it failed rn == NULL and the scanf will crash.
5) Avoid the scanf in the loop. Use the fgets/strtod combination.
6) The last scanf is asking for a string but storing it in a char. This is VERY dangerous. Use fgets and read the user input into a string. You can then check s[0] to see if it is Y or y.
7) The code would be more readable and not assume ASCII if you did:

    if(start == 'Y' || start == 'y')

8) Finally, for a small program like this the goto is not too bad but I'd want to get into the habit of using proper control structure. My logic would be:

    do {
        /* clear screen */
        /* ask how many resistors */
        rnumber = /* get answer */
        for( count = 0; count < rnumber; count++)
        {
             /* ask what is the value of resistor 'count' + 1 */
             double rn = /* get answer */
             rseries = rseries + rn;
             reff = reff + (1 / rn );
        }
        reff = 1/reff;

        /* print the results */

        do {
            /* ask if theyd like to do it again */
        } while( /* answer not yes or no */);
    } while(/* answer is yes */);

    printf("goodbye\n");
    return 0;
}

No need for the goto.
0

Featured Post

Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 4
  • 4
  • 3
  • +3
Tackle projects and never again get stuck behind a technical roadblock.
Join Now