Invert if/else/error: Difference between revisions

From Octave
Jump to navigation Jump to search
No edit summary
 
(5 intermediate revisions by one other user not shown)
Line 62: Line 62:
== Files ==
== Files ==


Total: 43
As of now, all the files have been corrected in Octave core source. Contributors are, however, welcomed to make changes as described above in Octave's packages.
 
Fixed: 6
 
{| class="wikitable"
!Owner !! File
 
|-
| jwe || corefcn/Cell.cc
|-
| jwe || corefcn/__ilu__.cc
|-
| jwe || corefcn/bitfcns.cc
|-
| jwe || corefcn/cellfun.cc
|-
| jwe || corefcn/data.cc
|-
| jwe || corefcn/debug.cc
|-
| jwe || corefcn/error.cc
|-
| jwe || <strike>corefcn/graphics.cc</strike>
|-
| jwe || <strike>corefcn/graphics.in.h</strike>
|-
| ??? || corefcn/input.cc
|-
| ??? || corefcn/load-save.cc
|-
| ??? || corefcn/ls-mat-ascii.cc
|-
| ??? || corefcn/ls-mat5.cc
|-
| ??? || corefcn/ls-oct-text.cc
|-
| ??? || corefcn/oct-map.cc
|-
| ??? || corefcn/oct-stream.cc
|-
| ??? || corefcn/sparse-xpow.cc
|-
| ??? || corefcn/symtab.h
|-
| ??? || corefcn/typecast.cc
|-
| ??? || corefcn/urlwrite.cc
|-
| das || <strike>corefcn/utils.cc</strike>
|-
| das || corefcn/variables.cc
|-
| jwe || <strike>dldfcn/chol.cc</strike>
|-
| jwe || <strike>octave-value/ov-base-diag.cc</strike>
|-
| jwe || <strike>octave-value/ov-base-int.cc</strike>
|-
| jwe || <strike>octave-value/ov-base-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-base-sparse.cc</strike>
|-
| jwe || <strike>octave-value/ov-base.cc</strike>
|-
| jwe || <strike>octave-value/ov-bool-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-cell.cc</strike>
|-
| jwe || <strike>octave-value/ov-class.cc</strike>
|-
| jwe || <strike>octave-value/ov-classdef.cc</strike>
|-
| jwe || <strike>octave-value/ov-cx-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-flt-cx-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-flt-re-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-perm.cc</strike>
|-
| jwe || <strike>octave-value/ov-re-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-str-mat.cc</strike>
|-
| jwe || <strike>octave-value/ov-struct.cc</strike>
|-
| jwe || <strike>octave-value/ov-usr-fcn.cc</strike>
|-
| jwe || <strike>octave-value/ov.cc</strike>
|-
| jwe || <strike>parse-tree/pt-assign.cc</strike>
|-
| jwe || <strike>parse-tree/pt-idx.cc</strike>
|-
 
|}
 
== Instances ==
 
Total: 104
 
Fixed: 0
 
<pre>
corefcn/graphics.in.h:1194
corefcn/symtab.h:151
corefcn/Cell.cc:306
corefcn/__ilu__.cc:767
corefcn/bitfcns.cc:111
corefcn/bitfcns.cc:506
corefcn/bitfcns.cc:509
corefcn/cellfun.cc:219
corefcn/cellfun.cc:686
corefcn/data.cc:658
corefcn/data.cc:838
corefcn/data.cc:1381
corefcn/data.cc:5620
corefcn/debug.cc:1383
corefcn/error.cc:1863
corefcn/graphics.cc:1313
corefcn/graphics.cc:1332
corefcn/graphics.cc:1365
corefcn/graphics.cc:1570
corefcn/graphics.cc:2514
corefcn/graphics.cc:2517
corefcn/graphics.cc:2540
corefcn/graphics.cc:10132
corefcn/graphics.cc:10634
corefcn/graphics.cc:10641
corefcn/graphics.cc:11454
corefcn/input.cc:761
corefcn/load-save.cc:748
corefcn/ls-mat-ascii.cc:322
corefcn/ls-mat-ascii.cc:343
corefcn/ls-mat-ascii.cc:347
corefcn/ls-mat-ascii.cc:351
corefcn/ls-mat5.cc:596
corefcn/ls-mat5.cc:1023
corefcn/ls-mat5.cc:2329
corefcn/ls-oct-text.cc:285
corefcn/oct-map.cc:277
corefcn/oct-stream.cc:1843
corefcn/sparse-xpow.cc:125
corefcn/sparse-xpow.cc:198
corefcn/typecast.cc:84
corefcn/typecast.cc:294
corefcn/urlwrite.cc:276
corefcn/utils.cc:1158
corefcn/utils.cc:1187
corefcn/variables.cc:174
corefcn/variables.cc:1499
dldfcn/chol.cc:216
dldfcn/chol.cc:241
octave-value/ov-base-diag.cc:528
octave-value/ov-base-int.cc:231
octave-value/ov-base-mat.cc:96
octave-value/ov-base-sparse.cc:464
octave-value/ov-base.cc:1614
octave-value/ov-base.cc:1632
octave-value/ov-bool-mat.cc:265
octave-value/ov-bool-mat.cc:294
octave-value/ov-cell.cc:809
octave-value/ov-cell.cc:818
octave-value/ov-cell.cc:848
octave-value/ov-cell.cc:863
octave-value/ov-cell.cc:963
octave-value/ov-class.cc:1272
octave-value/ov-class.cc:1275
octave-value/ov-class.cc:1707
octave-value/ov-class.cc:1710
octave-value/ov-classdef.cc:530
octave-value/ov-classdef.cc:558
octave-value/ov-classdef.cc:588
octave-value/ov-classdef.cc:1758
octave-value/ov-classdef.cc:2054
octave-value/ov-classdef.cc:2403
octave-value/ov-classdef.cc:2572
octave-value/ov-classdef.cc:3144
octave-value/ov-classdef.cc:3183
octave-value/ov-classdef.cc:3248
octave-value/ov-classdef.cc:3443
octave-value/ov-cx-mat.cc:324
octave-value/ov-cx-mat.cc:398
octave-value/ov-cx-mat.cc:422
octave-value/ov-flt-cx-mat.cc:298
octave-value/ov-flt-cx-mat.cc:372
octave-value/ov-flt-cx-mat.cc:396
octave-value/ov-flt-re-mat.cc:276
octave-value/ov-flt-re-mat.cc:399
octave-value/ov-flt-re-mat.cc:423
octave-value/ov-perm.cc:308
octave-value/ov-re-mat.cc:285
octave-value/ov-re-mat.cc:501
octave-value/ov-re-mat.cc:525
octave-value/ov-str-mat.cc:233
octave-value/ov-str-mat.cc:250
octave-value/ov-str-mat.cc:269
octave-value/ov-str-mat.cc:382
octave-value/ov-str-mat.cc:419
octave-value/ov-str-mat.cc:426
octave-value/ov-struct.cc:764
octave-value/ov-struct.cc:1403
octave-value/ov-usr-fcn.cc:158
octave-value/ov-usr-fcn.cc:162
octave-value/ov-usr-fcn.cc:989
octave-value/ov.cc:2995
parse-tree/pt-assign.cc:278
parse-tree/pt-idx.cc:524
</pre>

Latest revision as of 17:33, 24 December 2017

Introduction[edit]

The C++ error handling mechanism in core Octave has been changed to use exceptions. Previously, calling error() in C++ would always return execution to the calling function. This made it necessary to explicitly use the else keyword to control program flow. With exceptions, the C++ code may now be written in a manner that closely resembles Octave's own m-file language.

The principal work for this topic is to convert if/else statements that have an error() call in the else branch to a new format. The goal is to move the error() call to be after the if statement, eliminate the else branch, and decrease the indent of the remaining code. This requires reversing the conditional test in the if. For a single condition, the condition should be reversed rather than just adding the negation operator '!' to the start of the conditional. For a test with multiple conditions, use DeMorgan's Law (Demorgan's Law). In brief, you will need to negate each individual conditional and then change all && to || or vice versa.

Example 1 : Single Conditional (rows() from data.cc)[edit]

Before After
  bool do_set (const octave_value& v)
  {
    if (v.is_scalar_type () && v.is_real_type ())
      {
        double new_val = v.double_value ();

        if (new_val != current_val)
          {
            current_val = new_val;
            return true;
          }
      }
    else
      error ("set: invalid value for double property \"%s\"",
             get_name ().c_str ());
    return false;
  }
  bool do_set (const octave_value& v)
  {
    if (! v.is_scalar_type () || ! v.is_real_type ())
      error ("set: invalid value for double property \"%s\"",
             get_name ().c_str ());

    double new_val = v.double_value ();
  
    if (new_val != current_val)
      {
        current_val = new_val;
        return true;
      }

    return false;
  }

Detailed Instructions[edit]

The list of files which contain an instance of else followed by error() is shown in the Files section of this page. The actual instances, including line numbers, are shown in the Instances section. To avoid duplication, sign up for a particular file by editing the Files section of this wiki page and replacing '???' with your name. When you have edited a file you should verify that everything is okay by executing

make all
./run-octave
test FILENAME

When that passes, upload your patch to the patch tracker https://savannah.gnu.org/patch/?group=octave. If you can, e-mail the Octave Maintainer's list at octave-maintainers@gnu.org and let us know that there is something to review. Also, add the wiki tags

<strike> ... </strike>

to the Files section to cross the file off the list. In addition, increment the number of files that were fixed by +1.

Files[edit]

As of now, all the files have been corrected in Octave core source. Contributors are, however, welcomed to make changes as described above in Octave's packages.