Archetype for C++ function

[Rik]

12/6/15

jwe,

The switch to exceptions in the core is making the code a lot clearer, in
my mind anyways.  I want to propose a new archetype for C++ functions, and
if you like it, we can possibly have it as one of the code sprint topics.

The old archetype was

--- Archetype 1 ---
octave_value retval;
int nargin = args.length ();

if (nargin == 2)
  {
    NDArray x = args(0).array_value ();
    if (! error_state)
      {
        std::string opt = args(1).string_value ();
        
        if (! error_state)
          {
             // Do algorithm
             ...
             ...
             ...
             ...
          }
        else
          error ("second argument must be a string");
      }
    else
      error ("first argument must be a numeric array");
  }
else
  print_usage ();

return retval;
--- End Code ---

Now that error() short-circuits it makes sense to put the input validation
first.  But the declaration "octave_value retval;" was also part of this
structure because the original template would always reach the final
"return retval;" statement.  Since that is no longer true, I think it makes
sense to transition from the 'C' style of declaring all variables used
anywhere in the function at the top, to the 'C++' style of declaring
variables as they are needed in the narrowest possible local scope.  In
this same vein, I wouldn't declare variables unnecessarily and often
'nargin' is used just once in the input validation.  For these cases I
would just code "args.length ()" directly.  Here is the proposed new
archetype for the same code sample as above.

--- Archetype 2 ---
if (args.length () != 2)
  print_usage ();

NDArray x = args(0).xarray_value ("first argument must be a numeric array");
std::string opt = args(1).xstring_value ("second argument must be a string");

octave_value retval;

// Do algorithm
...
...
...
...

return retval;
--- End Code ---

This leads to some very tight pieces of code.  For example, the rows
function in data.cc is coded today as

--- rows DEFUN ---
if (args.length () != 1)
  print_usage ();

return octave_value (args(0).rows ());
--- End Code ---

Where args.length() is used multiple times I would define the variable
nargin since it makes the code more readable.  For example, the DEFUN for
complex() in data.cc is currently written as

--- complex DEFUN ---
int nargin = args.length ();

if (nargin < 1 || nargin > 2)
  print_usage ();

if (nargin == 1)
  {
    ...
    ...
  }
else
  {
    ...
    ...
  }
--- End Code ---

In browsing through the code I see there has been some confusion as to what
data type to use for nargin.  The two most common templates are

--- nargin templates ---
int nargin = args.length ();
octave_idx_type nargin = args.length ();
--- End Code ---

I propose standardizing on the first since it is shorter.

Finally, I think you had a preference in the m-files not to use length()
when what was actually desired was the number of elements.  Currently the
octave_value_list() class does not have a numel() function.  This is ironic
since the length() member function actually calls numel() internally. 
Would it make sense to add a numel member function so that one could write

int nargin = args.numel ();

--Rik