Thread: Is this the perfect input function?

Originally Posted by Thantos

IMO an input function should NEVER EVER output an error message. You should simply return a value that indicates an error and possibly what error and let the programmer decided what to do with it.

you mean like NULL or for example -1? if you mean a long value that indicates the error then lets say its -1 , what if the user entered -1 as input and an error occured , how would he know about it? he'd think that the function returned his value successfly ,right?

Another thing is when checking the range, the min value must be less than or equal to the max value. If these values are switched by accident, you will never get good input since a value cannot be less than 0 and greater than 500 in your example

fixed :

Code:

long getl(long min_int, long max_int, char *message)
{
   char input[BUFSIZ];
   char *p;
   long result , temp;

   if(max_int < min_int)
   {
     temp = max_int;
     max_int = min_int;
     min_int = temp;
   }

   for(;;)
   {

   if(fgets(input, sizeof(input), stdin)==NULL)
   {
       printf("\nCould not read from stream\n");
       return 0;
   }

   result = strtol(input, &p, 10);

      if(result <= max_int && result >= min_int && input[0]!='\n' && ( *p == '\0' || *p == '\n' ))
             return result;
      else
          puts(message);
   }
}

thanks for notifying me about this.

>Since 'scanf()' is really ugly dealing with inputs
Debatable. There's really nothing wrong with scanf as long as you know how and when to use it.

>i decided to write a function that will hopefully be perfect when asking for numbers from the user.
>The function is called 'getl()' (get long),
What if I want a float?

I don't really get your post thantos , whats the proper value to be returned on error?

I agree on the message thing though , the programmer should deal with the return value the way it fits him.

Originally Posted by Prelude

What if I want a float?

I'd add an extra argument to specify wether i want a float or an integer then use either 'strtol()' or 'strtod()' within the function depending on that argument. I just wanna discuss the concept of 'getl()' for now


sorry for the late response , i've been away for a while..

>I just wanna discuss the concept of 'getl()' for now
Okie dokie, let's discuss the issues:

1) Returning a long

The biggest problem with returning a long is that the only way to notify the client code of an error is by setting a global flag (henceforth to be known as "The errno effect") or to pass an error flag to the function, thus making it harder to call. If you want to return an error code and valid data in the same type then it takes more planning. For example, strtol returns a valid long, but also sets errno, and makes use of an end pointer. This isn't a bad idea until you try to write bulletproof code for strtol. It's tricky and convoluted.

So perhaps it would be a better idea to separate the error handling and the valid data handling. You can do this a number of ways, but one that seems to work well is to pass a variable to getl and return a status flag:

Code:

int getl ( long *val );

An alternative is to return a long, but also give the user the option of handling errors by accepting a null pointer in place of the flag:

Code:

long getl ( int *err );

Or you could go by the "don't worry, be happy" mantra and just let errors go undetected, but I don't recommend that. You could also set errno and use that.

In the end there's no good way around the problem, but for the rest of this discussion I'll assume that you prefer returning a status flag and taking a pointer to long as the argument.

2) Internal buffer

Presently your implementation uses an internal buffer. fgets and strtol are used to parse and convert the line of input into a valid long. This is bad for several reasons. The first reason is that fgets will take a line of input. In other words, if the client program wants to take an age and a name on the same line, tough. Do you really want your library functions to be making formatting decisions for you? I don't.

The second reason is that the buffer could be filled before a complete line is read. As we all know, fgets returns success even if the line is too long for the buffer and the only way to test for it is to look for a newline in the buffer. Ignoring the fact that you don't do that, consider the possible ramifications of what would happen. Well, it really depends on how you react to it. Do you try to read all of the line and brute force your way through the operation? Or do you terminate with an error and boo-hoo, a chunk of the user's input goes to never-never land?

IMO, an input function should only take what it absolutely needs to do the job, and leave everything else for other input functions to deal with. This tends to work best when you have several functions working in conjunction or when the input data is well formatted. We'll get to possible solutions in a moment.

3) Passing maximum and minimum limits as parameters

I don't like this because testing for limits is very often a client task. Sure, getl can do it and it will work nicely, but this goes against the concept of having a function do one operation. How about getl gets a long, and the client tests for other limits as needed? Much like strtol converts a long, but we almost always use it as an int.

The solution:

Okay, so how to implement getl? Well, we've worked out several ways of error handling and chosen one at random. We want to avoid a greedy input algorithm, so fgets is out, and we'll leave boundary checks to the client. Two immediate solutions come to mind: use getchar and fill a buffer, then use strtol as you did before; use scanf because it does all of the dirty work and pretty much handles what you want.

The first option is tricky because you need to consider how big the buffer can be. Do you want to read all digits and let strtol handle overflow on the assumption that this is what will be needed? Do you want to limit the buffer size to the exact number of characters that the largest long value can be? For a decent implementation, the former requires a resizable buffer and thus, another point of failure if allocation requests fail. The latter requires a tricky test to get the size of LONG_MAX. You also need to consider + and - as valid characters and make room for them as well as handle them accordingly.

While either of these are valid options, they imply too much work for too little gain. The other option is to use scanf. Now we have several questions about error handling again. Do we simply return failure if scanf fails and let the client code figure out what went wrong? This is certainly a viable option because feof and ferror are readily available. It also saves us the effort of developing several error codes, and thus we can simply use true and false.

Another question is how to deal with invalid input. Does getl want to remove invalid input and try again in an attempt to recover? Or does it just want to give the client more flexibility. Naturally the latter will open up opportunities for infinite loops where the client is too naive to remove invalid data from the stream before calling getl again. But that isn't nearly as bad as getl taking possibly valid input from the stream. For example, what if the client wanted to read either an age or a name and uses getl's return code to determine which it is? Flexibility is good, and you can't guess the legitimate uses for getl that the client will think up. Better to avoid recovery attempts.

With all of these questions and possible answers in mind, perhaps getl is best implemented like so?

Code:

int getl ( long *val )
{
  return scanf ( "%ld", val ) == 1;
}

Thanks for the great reply Prelude , i'd like to argue on some points though :

if the client program wants to take an age and a name on the same line, tough. Do you really want your library functions to be making formatting decisions for you? I don't.

Sorry but this wasn't clear for me , maybe an example of that situation would help ..

As we all know, fgets returns success even if the line is too long for the buffer and the only way to test for it is to look for a newline in the buffer. Ignoring the fact that you don't do that, consider the possible ramifications of what would happen.

Since the max input would be 11 digits (including the - and + )then what would the ramifications be? the user wouldn't enter a number larger\smaller than 'long' can handle because the program would store gibberish in the variable.

For example, what if the client wanted to read either an age or a name and uses getl's return code to determine which it is?

I don't this either but you don't really have to explain it lol (what you've done is really enough)

Code:

int getl ( long *val )
{
  return scanf ( "%ld", val ) == 1;
}

what if the user wanted to enter '444' but he typed 'r444' by mistake, it would return gibberish .. that wouldn't be so nice ,would it?


thanks so much for the reply

and sorry for the late respone .. i was busy with college

But it would return 1 for '555a5' and treat it as '555'

Originally Posted by Brain Cell

But it would return 1 for '555a5' and treat it as '555'

strtol, which is used in the "perfect version", has the same side effect:

Code:

#include <stdio.h>
#include <stdlib.h>
int main( void )
{
        char buf[BUFSIZ];
        long int l;
        printf("Number: ");
        fflush( stdout );
        fgets( buf, BUFSIZ, stdin );
        l = strtol( buf, NULL, 10L );
        printf("Output: %ld\n", l );

        return 0;
}

/*
~/programming/cboard$ ./stl
Number: 555d5
Output: 555
*/

Quzah.

well thats because you didn't take advantage of the second argument and used NULL instead. This works strictly :

Code:

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


int main(void)
{
   char buff[BUFSIZ], *p;
   long num;

   fgets(buff, sizeof(buff), stdin);

   num = strtol(buff, &p, 10);

      if( buff[0]!='\n' && ( *p=='\0' || *p=='\n'))
           printf("%d\n", num);
      else
           printf("Invalid input\n");

   return 0;
}

But Prelude is saying its not good within another function. How about if i keep repeating that in 'main' for every input? will it be better?

I think most of the points i'd like to make have already been made those being scanf when used how intended work fine. And "what if I want a double"
but more importantly: you forgot to make it platform independant.

Code:

system("PAUSE");

but you knew that already didn't you?
By the way, what's wrong with just accepting the input as a string and parsing it accordingly? I see nothing wrong with that, unless this exercise is one of intellectual curiousity.

>Sorry but this wasn't clear for me , maybe an example of that situation would help ..
Write a program using your getl that allows the user to type five numbers on a line, and then print out each of those five numbers. As it is you would read one number, and if that worked (which it won't thanks to your error checking...so ironic) then you would discard the others and be forced to ask again to get subsequent numbers.

>Since the max input would be 11 digits (including the - and + )then what would the ramifications be?
What about the rest of the input? What if the maximum value of a long is more than 11 digits?

>I don't this either but you don't really have to explain it lol
Sometimes it's useful to attempt to read a value and then attempt to read a value of another type if that fails. For example:

Code:

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

int main ( void )
{
  char name[BUFSIZ];
  int age;

  printf ( "Enter your name or age: " );
  fflush ( stdout );

  if ( scanf ( "%d", &age ) == 1 )
    printf ( "You're %d years old?\n", age );
  else {
    if ( fgets ( name, sizeof name, stdin ) != NULL )
      printf ( "Nice to meet you, %s", name );
    else
      fprintf ( stderr, "Error processing input\n" );
  }
}

>it would return gibberish .. that wouldn't be so nice ,would it?
It isn't the job of the input function to validate data. A generic input function gets data of a certain type and hands it over to the client code for processing. If an input error occurs or data of the wrong type is encountered, that's a valid problem and only the client code can know how to react.

>But it would return 1 for '555a5' and treat it as '555'
How do you know that this isn't the correct behavior? Once again, input functions know nothing about the application's expected data formatting, and thus can't assume anything.

>But Prelude is saying its not good within another function.
It depends on the function. But with your function, it causes more problems than it solves.

>How about if i keep repeating that in 'main' for every input? will it be better?
If you find yourself repeating it then write a wrapper for the application. If you put it in getl itself then the chances for reuse are cut drastically because the function is not as flexible.