Re: Cleaning up C++ input validation of strings

[Rik]

On 02/16/2015 04:43 AM, Carnë Draug wrote:
> 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ë

That is the correct general pattern, but not for the input validation of
string option arguments.  As an example, in this particular case the option
string "vector" means do something different.  If you just ask for a
string_value then the double matrix

[118   101    99   116   111   114]

which spells out "vector" would also be accepted.  But if you are supplying
numeric values like this you haven't read the docstring and are probably
misusing the function.  We can't program Octave with a full Artificial
Intelligence agent, but we would like to cut short as quickly as possible
any Garbage In/Garbage Out calculations by validating the inputs as closely
as possible.

--Rik