Refactor C++ code that uses print usage() to resemble m-files

From Octave
Revision as of 14:11, 12 December 2015 by 73.223.125.174 (talk) (→‎Files)
Jump to navigation Jump to search

Introduction

The C++ error handling mechanism in core Octave has been changed to use exceptions. Previously, calling print_usage() or error() in C++ would always return execution to the calling function. This made it necessary to explicitly use the return keyword to exit a function, or arrange for an if/else tree in the original function so that normal function execution would not continue in the case of an error. 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 sprint topic is to convert the C++ code to the new syntax. In general, this means moving the input validation to the start of the function and logically negating the conditional used to test for an error condition. For a single condition, the condition itself 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)

Before After
  octave_value retval;

  if (args.length () == 1)
    retval = args(0).rows ();
  else
    print_usage ();

  return retval;
  octave_value retval;

  if (args.length () != 1)
    print_usage ();

  retval = args(0).rows ();

  return retval;

Example 2 : Multiple Conditional (fopen() from file-io.cc)

Before After
  if (nargin > 0 && nargin < 4)
    {
      octave_value mode = (nargin == 2 || nargin == 3)
                          ? args(1) : octave_value ("r");

      octave_value arch = (nargin == 3)
                          ? args(2) : octave_value ("native");

      int fid = -1;

      octave_stream os = do_stream_open (args(0), mode, arch, "fopen", fid);

      if (os)
        {
          retval(1) = "";
          retval(0) = octave_stream_list::insert (os);
        }
      else
        {
          int error_number = 0;

          retval(1) = os.error (false, error_number);
          retval(0) = -1.0;
        }
    }
  else
    print_usage ();
  if (nargin < 1 || nargin > 4)
    print_usage ();

  octave_value mode = (nargin == 2 || nargin == 3)
                      ? args(1) : octave_value ("r");

  octave_value arch = (nargin == 3)
                      ? args(2) : octave_value ("native");

  int fid = -1;

  octave_stream os = do_stream_open (args(0), mode, arch, "fopen", fid);

  if (os)
    {
      retval(1) = "";
      retval(0) = octave_stream_list::insert (os);
    }
  else
    {
      int error_number = 0;

      retval(1) = os.error (false, error_number);
      retval(0) = -1.0;
    }

Example 3 : Redundant return statement

Before After
  octave_value_list retval;

  int nargin = args.length ();

  // verify arguments
  if (nargin != 2 && nargin != 4)
    {
      print_usage ();
      return retval;
    }
  octave_value_list retval;

  int nargin = args.length ();

  // verify arguments
  if (nargin != 2 && nargin != 4)
    print_usage ();

Detailed Instructions

The list of files which contain an instance of print_usage() 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, let a Maintainer know so that we can check in the changes. 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

Start of Sprint

Total: 7

Fixed: 0

Owner File
Yu Liu corefcn/syscalls.cc
??? corefcn/data.cc
??? corefcn/file-io.cc
??? dldfcn/chol.cc
??? dldfcn/qr.cc
??? dldfcn/audiodevinfo.cc
??? octave.cc

Instances

Start of Sprint

Total: 83

Fixed: 0

corefcn/syscalls.cc:643:    print_usage ();
corefcn/syscalls.cc:662:    print_usage ();
corefcn/syscalls.cc:708:    print_usage ();
corefcn/syscalls.cc:735:    print_usage ();
corefcn/syscalls.cc:816:    print_usage ();
corefcn/syscalls.cc:882:    print_usage ();
corefcn/syscalls.cc:1005:    print_usage ();
corefcn/syscalls.cc:1028:    print_usage ();
corefcn/syscalls.cc:1051:    print_usage ();
corefcn/syscalls.cc:1074:    print_usage ();
corefcn/syscalls.cc:1097:    print_usage ();
corefcn/syscalls.cc:1120:    print_usage ();
corefcn/syscalls.cc:1143:    print_usage ();
corefcn/syscalls.cc:1166:    print_usage ();
corefcn/syscalls.cc:1182:    print_usage ();
corefcn/syscalls.cc:1231:    print_usage ();
corefcn/syscalls.cc:1266:    print_usage ();
corefcn/syscalls.cc:1347:    print_usage ();
corefcn/syscalls.cc:1369:    print_usage ();
corefcn/syscalls.cc:1413:    print_usage ();
corefcn/syscalls.cc:1437:    print_usage ();
corefcn/syscalls.cc:1463:    print_usage ();
corefcn/syscalls.cc:1488:    print_usage ();
corefcn/syscalls.cc:1512:    print_usage ();
corefcn/syscalls.cc:1534:    print_usage ();
corefcn/syscalls.cc:1562:    print_usage ();
corefcn/syscalls.cc:1577:    print_usage ();
corefcn/data.cc:2414:    print_usage ();
corefcn/data.cc:2584:    print_usage ();
corefcn/data.cc:2737:    print_usage ();
corefcn/data.cc:2823:    print_usage ();
corefcn/data.cc:7805:    print_usage ();
corefcn/data.cc:7823:    print_usage ();
corefcn/data.cc:7926:    print_usage ();
corefcn/file-io.cc:684:    print_usage ();
dldfcn/chol.cc:695:    print_usage ();
dldfcn/chol.cc:881:    print_usage ();
dldfcn/chol.cc:1110:    print_usage ();
dldfcn/chol.cc:1249:    print_usage ();
dldfcn/qr.cc:1069:    print_usage ();
dldfcn/qr.cc:1271:    print_usage ();
/* These should possibly be using error rather than print_usage.
   They are internal functions and print_usage() will print nothing valuable */ 
dldfcn/audiodevinfo.cc:2010:    print_usage ();
dldfcn/audiodevinfo.cc:2038:        print_usage ();
dldfcn/audiodevinfo.cc:2067:        print_usage ();
dldfcn/audiodevinfo.cc:2096:        print_usage ();
dldfcn/audiodevinfo.cc:2125:        print_usage ();
dldfcn/audiodevinfo.cc:2154:        print_usage ();
dldfcn/audiodevinfo.cc:2183:        print_usage ();
dldfcn/audiodevinfo.cc:2212:        print_usage ();
dldfcn/audiodevinfo.cc:2241:        print_usage ();
dldfcn/audiodevinfo.cc:2270:        print_usage ();
dldfcn/audiodevinfo.cc:2299:        print_usage ();
dldfcn/audiodevinfo.cc:2326:    print_usage ();
dldfcn/audiodevinfo.cc:2353:      print_usage ();
dldfcn/audiodevinfo.cc:2389:        print_usage ();
dldfcn/audiodevinfo.cc:2418:        print_usage ();
dldfcn/audiodevinfo.cc:2447:        print_usage ();
dldfcn/audiodevinfo.cc:2476:        print_usage ();
dldfcn/audiodevinfo.cc:2503:    print_usage ();
dldfcn/audiodevinfo.cc:2531:    print_usage ();
dldfcn/audiodevinfo.cc:2536:    print_usage ();
dldfcn/audiodevinfo.cc:2610:        print_usage ();
dldfcn/audiodevinfo.cc:2639:        print_usage ();
dldfcn/audiodevinfo.cc:2668:        print_usage ();
dldfcn/audiodevinfo.cc:2697:        print_usage ();
dldfcn/audiodevinfo.cc:2726:        print_usage ();
dldfcn/audiodevinfo.cc:2755:        print_usage ();
dldfcn/audiodevinfo.cc:2784:        print_usage ();
dldfcn/audiodevinfo.cc:2813:        print_usage ();
dldfcn/audiodevinfo.cc:2842:        print_usage ();
dldfcn/audiodevinfo.cc:2871:        print_usage ();
dldfcn/audiodevinfo.cc:2900:    print_usage ();
dldfcn/audiodevinfo.cc:2965:        print_usage ();
dldfcn/audiodevinfo.cc:3030:        print_usage ();
dldfcn/audiodevinfo.cc:3059:        print_usage ();
dldfcn/audiodevinfo.cc:3088:        print_usage ();
dldfcn/audiodevinfo.cc:3117:        print_usage ();
dldfcn/audiodevinfo.cc:3146:        print_usage ();
octave.cc:240:    print_usage ();
octave.cc:1022:    print_usage ();
octave.cc:1057:    print_usage ();
octave.cc:1084:    print_usage ();
octave.cc:1107:    print_usage ();