[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Archetype for C++ function
From: |
Rik |
Subject: |
Archetype for C++ function |
Date: |
Sun, 6 Dec 2015 20:07:22 -0800 |
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