Re: Cleaning up C++ input validation of strings

[Carnë Draug]

On 14 February 2015 at 01:09, Rik <address@hidden> wrote:
> 2/13/15
>
> There seem to be multiple instances where we check whether an input is a
> string, then extract a string_value, and then check the error_status to
> make sure that the string_value was extracted correctly.  This seems
> redundant and possibly a lazy copying of a correct pattern.
>
> If something might not be of type A, but possibly could be converted to
> type A, then asking for A_value () and checking the error_status is the
> correct thing to do.  In this case, If something is determined to be a
> string, can the string_value() call really fail?
>
> Here is an example from lu.cc
>
> -- Code --
> if (args(n).is_string ())
>   {
>     std::string tmp = args(n++).string_value ();
>
>     if (! error_state)
>       {
>         if (tmp.compare ("vector") == 0)
>           vecout = true;
>         else
>           error ("lu: unrecognized string argument");
>       }
>   }
> -- End Code --
>
> I propose simplifying to
>
> -- Code --
> if (args(n).is_string ())
>   {
>     std::string tmp = args(n++).string_value ();
>
>     if (tmp.compare ("vector") == 0)
>       vecout = true;
>     else
>       error ("lu: unrecognized string argument");
>   }
> -- End Code --
>
> Are there any objections?

I was under the impression that we should avoid using is_* functions as
much as possible and instead check error_state.  Following this logic,
should not the recommendation be:

-- Code --
    std::string tmp = args(n).string_value ();
    if (! error_state)
      {
        n++;
        if (tmp.compare ("vector") == 0)
          vecout = true;
        else
          error ("lu: unrecognized string argument");
      }
-- End Code --

Carnë