On Wed, Sep 30, 2015 at 04:58:22PM -0400, John W. Eaton wrote:
> On 09/28/2015 10:42 AM, Jordi Gutiérrez Hermoso wrote:
> >On Sun, 2015-09-27 at 10:38 +0200, Olaf Till wrote:
>
> >>What about providing
> >>
> >>#define SOME_MACRO(code, informative_error_message) \
> >>...
> >
> >Death to macros!
> >
> >In this case it's not so onerous to do
> >
> > try
> > {
> > std::string val = args(0).string_value ();
> > }
> > catch (invalid_value& e)
> > {
> > error ("foobar: first argument must be a character string");
> > }
> >
> >In general, replacing if(! error_state) with try-catch blocks is not
> >that complicated, but it will require changing a lot of calling sites.
>
> I think we also want to avoid adding many try-catch blocks if possible.
>
> In this case, instead of a macro, maybe an in-line function like
>
> inline std::string
> get_string (const octave_value& arg, const std::string& msg = std::string
> ())
> {
> std::string retval;
>
> try
> {
> retval = arg.string_value ();
> }
> catch
> {
> if (! msg.empty ())
> error ("%s", msg.c_str ());
> else
> throw;
> }
>
> return retval;
> }
>
> Then we could just write
>
> std::string val = get_string (args(0), "foobar: first argument must be a
> character string");
>
> and we would (currently) see something like
>
> error: octave_base_value::convert_to_str_internal (): wrong type argument
> 'matrix'
> error: foobar: first argument must be a character string
>
> It would probably be good to take this opportunity to modify the behavior to
> either omit the wrong-type argument message, or at least clean it up so that
> it doesn't print the "octave_base_value::convert_to_str_internal ()" part.
>
> In any case, this seems cleaner and clearer than having either the try-catch
> block repeated many times or writing
>
> if (! args(0).is_string ())
> {
> error ("foobar: first argument must be a character string");
> return retval;
> }
>
> std::string val = args(0).string_value ();
>
> I'm not sure whether I would have this as a simple function or make it a
> method in the otave_value class. I suppose it could just be an overloaded
> version of the octave_value::string_value method.
Overloading the ..._value() methods seems good to me if this way is
gone. But of course in this way you only handle the foreseen cases,
mostly ..._value() conversions, and you have to do it for each
separately ... still it would probably spare a good deal of 'outer'
try-catch blocks ...
As for testing with is_...() before conversion, I'd agree it isn't
very nice.
Olaf