Determining which command line arguments are numeric

I am working on using STL and in general safe C++ practices. Instead of posting large samples of code I am working with a small sample to catch problems before I move onto larger ones.

How do you find out if an istringstream found an input that is not of the desired type? That is, you have a string and you want to verify if it is a float or not.

Using the program below, when I input

[mehoggan@desktop argument_processing]$ ./args 1 2 3 4 2y

I get:

1 2 3 4 2

which is not what I want, since not all arguments where numeric.

I should of gotten:

ERROR: 2y is not numeric

What improvements would you make to this small program?

#include 
#include 
#include 
#include 
#include 

namespace CLP {
    using std::vector;
    using std::istringstream;
    using std::string;
    using std::cerr;
    using std::endl;

    vector all_numeric_arguments( int argc, char *argv[ ] ) {
        vector v_argv;
        vector v_tmp;
        copy( &argv[1],&argv[argc],back_inserter( v_tmp ) );
        for( vector::iterator it=v_tmp.begin( );it!=v_tmp.end( );it++ ) {
            istringstream iss( (*it) );
            float f=0.0;
            if( iss >> f >> std::dec ) {
                v_argv.push_back( f );
            } else {
               cerr  v = CLP::all_numeric_arguments( argc, argv );
    if( v.size( ) == (unsigned)(argc-1) ) {
        copy( v.begin( ),v.end( ),std::ostream_iterator( std::cout," " ) );
        std::cout << std::endl;
    }
}

In your case, you apparently want to verify that there was not only a successful conversion, but also that the conversion consumed all the input. Given that this is exactly what Boost lexical_cast does, my immediate advice would be to use its existing code to do the job. If you can't do that, I'd still write a similar function, and probably give it the same name, so people who already know about lexical_cast will be able to recognize what you're doing without having to read through the code to do it.

As far as other changes go, rather than using std::copy to copy argv into v_tmp, I'd probably just initialize v_tmp from argv. I'd also note that &x[y] is generally equivalent to x+y, and simplify that part of the code a bit, giving:

std::vector v_tmp(argv+1, argv+argc);

That gives us code that looks like:

template
Target lexical_cast(const Source& arg) { 
    // ...
}

std::vector v_tmp(argv+1, argv+argc);
vector v_argv;

for (int i=0; i(v_tmp[i]);

That does leave one real question, however: exactly how you want to signal the fact that your lexical_cast has failed. Boost lexical_cast throws an exception. One possible alternative to that would be to return a NaN instead. I'm not at all sure I'd recommend that, but I can imagine situations where it might be the preferred way to go.

Assuming you go the route of throwing an exception, the code would change to something like this:

for (int i=0; i(v_tmp[i]));
    }
    catch (bad_lexical_cast const &) {
        std::cerr << "Error: " << v_tmp[i] << " is not numeric.\n";
    }
}

That leaves us with the definition of lexical_cast itself. Most of it would quite obviously be essentially identical to the code you had, just adding a check that after we do the conversion, the stream is empty:

template
Target lexical_cast(const Source& arg) {      
    istringstream iss(arg);
    Target f = Target(); // generic equivalent to your `0.0` initialization

    iss >> f;
    char c;

    // if the conversion failed, or we can read another character, we 
    // couldn't convert, or there's data left in the stream, which we 
    // consider a failure.
    if (iss.fail() || iss.get(c))
        throw bad_lexical_cast();
    return f;
}

Since that's throwing a bad_lexical_cast, we'll also have to define that type. In this case, we'll keep it simple:

class bad_lexical_cast {};

In reality, you usually want to derive your exception classes (directly or indirectly) from std::exception (unless you've defined a similar hierarchy of your own -- in any case you typically want a common base class for all your exceptions). You probably also want to include an indication of what failed and why in the exception object itself, so the code that catches it doesn't need as much information about what was being done at the time as we used here.

StackExchange Code Review Q#3857, answer score: 7