Re: C bug of the day

[kanze@gabi-soft.fr]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 5046 bytes --]

Francis Glassborow <francis.glassborow@ntlworld.com> wrote in message
news:<jhPtsFBx6F7+EwOv@robinton.demon.co.uk>...
> In message <CnRGa.11791$KF1.273607@amstwist00>, Terje Sletteb�
> <tslettebo@chello.no.nospam> writes

> >I would be interested to know how read() is implemented. Could you
> >posted it?

> Well for what they are worth here is the complete set of overloaded
> function templates

> Comments for improving them are very welcome.

> // These templates provide safe input for any type that supports
> // operator>>

> // 1) from console with prompt

>         template<typename in_type>
>         in_type read(std::string const & prompt, int max_tries = 3){
>                 in_type temp;
>                 int tries(0);
>                 while(tries++ != max_tries ){
>                         std::cout << prompt;
>                         std::cin >> temp;
>                         if(std::cin.good()) return temp;
>                         std::cin.clear();       // if it has failed,
> reset it to normal
>                         while(cin.get() != '\n'); flush stream

I'm sure you meant to have a //  before the last two words:-).  And an
std:: before cin.

Also, although it is only a question of style, I would never write an
empty loop with just a ';' hidden on the same line as the while.

>                         std::cout << "\n That input was incorrect, try
> again: \n";
>                 }
>                 throw fgw::problem("Too many attempts to read data.");
>         }

I get an endless loop with this one.  Using g++ 3.2.2, under Solaris
2.8, I instantiating it with:

    int in( read< int >( "value: " ) ) ;

I entered "123", then control-D.  Valid input for an int, I believe, so
I shouldn't have looped at all.  (For those unfamiliar with Unix,
control-D causes an end of file on keyboard input.)

There are actually two errors (in addition to the typos in the line with
the while):

    if ( std::cin.good() ) return temp ;

The function good() takes the eofbit into consideration.  It is thus
completely worthless and unusable.  (Almost: if good() returns false,
you know that future input will fail.  You know nothing about past
input.  And if good() returns true, you know nothing, period.)  The
correct idiom here would be:

    if ( std::cin ) return temp ;

The second problem is with:

    while ( cin.get() != '\n' ) ...

if end of file or any error occurs, you have an endless loop.  As an
unrepentant Pascal programmer, I would write:

    while ( std::cin.peek() != EOF && std::cin.peek() != '\n') ...

More typical C++ idioms might be:

    char                ch ;
    while ( cin.get( ch ) && ch != '\n' ) ...

or:

    int                 ch ;
    while ( (ch = cin.get()) != EOF && ch != '\n' ) ...

or, if you don't like uninitialized variables (as I don't):

    for ( int ch = cin.get() ; 
              ch != EOF && ch != '\n' ; 
              ch = cin.get() ) ...

> // 2) version without prompt, defaults to colon space prompt
>         template<typename in_type>
>         in_type read(int max_tries=3){
>                 return read<in_type>(": ", max_tries);
>         }

> // 3) version for general input stream, throws an exception if fails
>         template<typename in_type>
>                 in_type read(std::istream & in){
>                         in_type temp;
>                         in >> temp;
>                         if(in.good()) return temp;
>                         if(in.eof()) throw fgw::problem("Tried to read
> past end of file"); // handle end of file as special case
>                         in.clear();  // reset input stream
>                         throw fgw::problem("Corrupted data stream.");
>                 }

And this has the same problem as the preceding: it "fails" for valid
input if the type requires look-ahead (most types do) and the input is
not followed by any white space character.

While the typos and the endless loop in the first bit of code are just
oversights, and could happen to anyone, the misuse of the function is
interesting.  It's not as if you are a beginner, who just looked for a
function with a likely name.  You're quite familiar with the behavior of
iostream.  I'd consider this an embarassing point in the standard, and
something worth correcting if someone can figure out how without
breaking existing code.

(BTW: I don't have anything similar.  I've never felt it necessary --
it's very rare that I will want to initialize a variable from a stream,
except in small, parser functions.  Like what you offer here.  And I
would consider myself a little more than just halfway competent.)

--
James Kanze             GABI Software             mailto:kanze@gabi-soft.fr
Conseils en informatique orient�e objet/
                           Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, T�l. : +33 (0)1 30 23 45 16

      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]