Re: Success of 2015 Code Sprint

[Rik]

On 12/19/2015 11:06 AM, Andreas Weber wrote:
> Am 15.12.2015 um 21:11 schrieb Rik: >> We had a very successful code sprint last Saturday.  Thanks to everyone >> involved we managed to tackle and complete 5 of 6 topics.  Should we make >> this a more regular thing, maybe semi-annual instead of just annual? > > Hi Rik. First thank you for the great preparation (for example the wiki > pages). I think we should make a Code Spint more often, 2 or 4 a year. > Perhaps we could also make a list with easy clean-up tasks which can be > done without a CodeSprint? > > For example removing the > > bool success = true; > ... > { >   error (..) >   success = false; > } > ... > return success; > > constructs. > > Or is there a reason to keep these? > > They appear in > octave-value/ov-flt-cx-mat.cc:368: > octave-value/ov-flt-cx-mat.cc:374: > octave-value/ov-flt-cx-mat.cc:380: > octave-value/ov-flt-cx-mat.cc:399: > octave-value/ov-flt-cx-mat.cc:410: > octave-value/ov-flt-cx-mat.cc:419: > octave-value/ov-base-sparse.cc:460: > octave-value/ov-base-sparse.cc:468: > octave-value/ov-base-int.cc:226: > octave-value/ov-base-int.cc:234: > octave-value/ov-base-diag.cc:514: > octave-value/ov-base-diag.cc:532: > octave-value/ov-flt-re-mat.cc:395: > octave-value/ov-flt-re-mat.cc:401: > octave-value/ov-flt-re-mat.cc:407: > octave-value/ov-flt-re-mat.cc:426: > octave-value/ov-flt-re-mat.cc:437: > octave-value/ov-flt-re-mat.cc:446: > octave-value/ov-cx-mat.cc:394: > octave-value/ov-cx-mat.cc:400: > octave-value/ov-cx-mat.cc:406: > octave-value/ov-cx-mat.cc:425: > octave-value/ov-cx-mat.cc:436: > octave-value/ov-cx-mat.cc:445: > octave-value/ov-str-mat.cc:375: > octave-value/ov-str-mat.cc:384: > octave-value/ov-str-mat.cc:390: > octave-value/ov-str-mat.cc:417: > octave-value/ov-str-mat.cc:444: > octave-value/ov-str-mat.cc:480: > octave-value/ov-perm.cc:297: > octave-value/ov-perm.cc:312: > octave-value/ov-bool-mat.cc:262: > octave-value/ov-bool-mat.cc:269: > octave-value/ov-bool-mat.cc:275: > octave-value/ov-bool-mat.cc:301: > octave-value/ov-bool-mat.cc:312: > octave-value/ov-bool-mat.cc:321: > octave-value/ov-re-mat.cc:497: > octave-value/ov-re-mat.cc:503: > octave-value/ov-re-mat.cc:509: > octave-value/ov-re-mat.cc:528: > octave-value/ov-re-mat.cc:539: > octave-value/ov-re-mat.cc:548: > octave-value/ov-fcn-handle.cc:283: > octave-value/ov-fcn-handle.cc:313: > octave-value/ov-fcn-handle.cc:336: > octave-value/ov-fcn-handle.cc:346: > octave-value/ov-fcn-handle.cc:1158: > octave-value/ov-fcn-handle.cc:1169: > octave-value/ov-struct.cc:759: > octave-value/ov-struct.cc:770: > octave-value/ov-struct.cc:881: > octave-value/ov-struct.cc:1407: > octave-value/ov-struct.cc:1418: > octave-value/ov-struct.cc:1491: >
12/21/15

Andreas,

Thanks for noticing these.  There were a lot of places where there was code following an error statement that was effectively unreachable and could therefore be deleted.  Most of these could be detected by fairly simple pattern recognition scripts in Perl.  I wrote a few of these and automatically converted most of the code base.  See csets 20952, 20953, 20956, and 20957.  There is another conversion project that I want to do which is reversing the order of input validation to move the error statement close to the test condition which provokes the error.  I find this more readable and it allows the indent level to be reduced.

For example,

bool do_set (const octave_value& newval)
{
  if (newval.is_string ())
    {
      std::string s = newval.string_value ();

      std::string match;

      if (vals.validate (s, match))
        {
          if (match != current_val)
            {
              if (s.length () != match.length ())
                warning_with_id ("Octave:abbreviated-property-match",
                                 "%s: allowing %s to match %s value %s",
                                 "set", s.c_str (), get_name ().c_str (),
                                 match.c_str ());
              current_val = match;
              return true;
            }
        }
      else
        error ("set: invalid value for radio property \"%s\" (value = %s)",
               get_name ().c_str (), s.c_str ());
    }
  else
    error ("set: invalid value for radio property \"%s\"",
           get_name ().c_str ());
  return false;
}

can be transformed to

bool do_set (const octave_value& newval)
{
  if (! newval.is_string ())
    error ("set: invalid value for radio property \"%s\"",
           get_name ().c_str ());

  std::string s = newval.string_value ();

  std::string match;

  if (! vals.validate (s, match))
    error ("set: invalid value for radio property \"%s\" (value = %s)",
           get_name ().c_str (), s.c_str ());

  if (match != current_val)
    {
      if (s.length () != match.length ())
        warning_with_id ("Octave:abbreviated-property-match",
                         "%s: allowing %s to match %s value %s",
                         "set", s.c_str (), get_name ().c_str (),
                         match.c_str ());
      current_val = match;
      return true;
    }

  return false;
}

With simple conditionals like the one above, I think I can write a script to do the conversion.  But multiple tests (A && B, A || (B && C)) are going to require human intervention.  When I have a list of those I am going to post those as one of the Short Projects (http://wiki.octave.org/Short_projects) on the Wiki.  I would really appreciate help at that point finishing off the conversion to the new function format.

--Rik