Is this the perfect input function?

This is a discussion on Is this the perfect input function? within the C Programming forums, part of the General Programming Boards category; IMO an input function should NEVER EVER output an error message. You should simply return a value that indicates an ...

  1. #16
    & the hat of GPL slaying Thantos's Avatar
    Join Date
    Sep 2001
    Posts
    5,681
    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.

  2. #17
    Registered User
    Join Date
    Oct 2004
    Posts
    120
    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

    And I agree with Thantos, let the caller of the function output the appropriate error message. An exception in your case, maybe if the range check fails you can print out the valid range as well as the user defined error message, but that could be left to the user to define in their error message as well!

  3. #18
    former member Brain Cell's Avatar
    Join Date
    Feb 2004
    Posts
    472
    Quote 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.
    My Tutorials :
    - Bad programming practices in : C
    - C\C++ Tips
    (constrcutive criticism is very welcome)


    - Brain Cell

  4. #19
    & the hat of GPL slaying Thantos's Avatar
    Join Date
    Sep 2001
    Posts
    5,681
    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?
    Which is why the conversion functions are not input functions. Also giving an error message to the screen still doesn't tell the programmer that there is an error.

  5. #20
    Registered User
    Join Date
    Sep 2004
    Location
    California
    Posts
    3,246
    I would change it to:

    Code:
    /*
     * Function returns 0 on error, or positive value on success.
     */
    int getl(long *lpinput, 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)
          {
             return 0;
          }
    
          result = strtol(input, &p, 10);
    
          if(result <= max_int && result >= min_int && input[0]!='\n' && ( *p == '\0' || *p == '\n' ))
          {
                 *lpinput = result;
                 return 1;
          }
          else
              puts(message);
       }
    }

  6. #21
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    >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?
    My best code is written with the delete key.

  7. #22
    former member Brain Cell's Avatar
    Join Date
    Feb 2004
    Posts
    472
    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.

    Quote 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..
    My Tutorials :
    - Bad programming practices in : C
    - C\C++ Tips
    (constrcutive criticism is very welcome)


    - Brain Cell

  8. #23
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    >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;
    }
    My best code is written with the delete key.

  9. #24
    former member Brain Cell's Avatar
    Join Date
    Feb 2004
    Posts
    472
    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
    My Tutorials :
    - Bad programming practices in : C
    - C\C++ Tips
    (constrcutive criticism is very welcome)


    - Brain Cell

  10. #25
    & the hat of GPL slaying Thantos's Avatar
    Join Date
    Sep 2001
    Posts
    5,681
    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?
    But the programmer would know that there is gibberish because of scanf()'s return value wouldn't be 1 thus you would get a return value from getl() of 0.

  11. #26
    former member Brain Cell's Avatar
    Join Date
    Feb 2004
    Posts
    472
    But it would return 1 for '555a5' and treat it as '555'
    My Tutorials :
    - Bad programming practices in : C
    - C\C++ Tips
    (constrcutive criticism is very welcome)


    - Brain Cell

  12. #27
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Quote 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.
    Hope is the first step on the road to disappointment.

  13. #28
    former member Brain Cell's Avatar
    Join Date
    Feb 2004
    Posts
    472
    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?
    My Tutorials :
    - Bad programming practices in : C
    - C\C++ Tips
    (constrcutive criticism is very welcome)


    - Brain Cell

  14. #29
    Registered User caroundw5h's Avatar
    Join Date
    Oct 2003
    Posts
    751
    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.

  15. #30
    Code Goddess Prelude's Avatar
    Join Date
    Sep 2001
    Posts
    9,796
    >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.
    My best code is written with the delete key.

Page 2 of 3 FirstFirst 123 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. In over my head
    By Shelnutt2 in forum C Programming
    Replies: 1
    Last Post: 07-08-2008, 06:54 PM
  2. Undefined Reference Compiling Error
    By AlakaAlaki in forum C++ Programming
    Replies: 1
    Last Post: 06-27-2008, 11:45 AM
  3. <Gulp>
    By kryptkat in forum Windows Programming
    Replies: 7
    Last Post: 01-14-2006, 12:03 PM
  4. c++ linking problem for x11
    By kron in forum Linux Programming
    Replies: 1
    Last Post: 11-19-2004, 09:18 AM
  5. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21