Re: Cleaning up C++ input validation of strings

[Carnë Draug]

On 16 February 2015 at 15:43, Rik <address@hidden> wrote:
> 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.
>

But isn't that the whole point of the num-to-str warning?  The
string_value() method even has a force option so as to not trigger
the warning.  Shouldn't error_state be set if force is false?

Carnë