Refactor C++ code to use ovl() when returning multiple values: Difference between revisions

Jump to navigation Jump to search

Refactor C++ code to use ovl() when returning multiple values (edit)

Revision as of 19:34, 14 December 2015

2,057 bytes added ,  14 December 2015
→‎Files
m (Update total number of instances)
 
(24 intermediate revisions by 2 users not shown)
Line 1: Line 1:
== Introduction ==
== Introduction ==


Some functions In Octave 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.
Some Octave functions return more than one value.  For example, the LU factorization can return L and U, but also P, Q, and R.  In C++ these functions return an octave_value_list object which is a row vector of return values.  The first element in the vector (element 0) contains the leftmost return argument.  For return arguments L,U,P the indices would be 0,1,2.


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 ([https://en.wikipedia.org/wiki/Demorgans_law Demorgan's Law]).  In brief, you will need to negate each individual conditional and then change all && to || or vice versa.
However, unless the octave_value_list has been pre-declared to the correct size, growing the row vector by indexing into a non-existent position forces the array to grow.  Thus, the straightforward and very clear coding strategy of  


==== Example 1 : Simple Conditional (colloc() from colloc.cc) ====
<pre>
retval(0) = L;
retval(1) = U;
retval(2) = P;
</pre>
 
turns out to be an inefficient implementation.  For this reason, current Octave code always writes this in reverse as
 
<pre>
retval(2) = P;
retval(1) = U;
retval(0) = L;
</pre>
 
The first indexing operation resizes the row vector and is done only once, but the disadvantage is that this code is more difficult to match with the function's behavior.
 
As an alternative, there is the ovl() function where the name stands for Octave Value List.  This function grows the vector just once and allows the arguments to be presented in their natural order.  In this case, the example code could be written as
 
<pre>
retval = ovl (L, U, P);
</pre>
 
The principal work for this sprint topic is to convert the C++ code to use the ovl() function where possible.  In general, this means collapsing multiple assignment statements to a single assignment.  Keeping in mind that line length should be limited to 80 characters, this may mean appropriately breaking the line and indenting the following lines.  In some cases, the declaration of "octave_value_list retval" may no longer be necessary and a direct "return ovl (...)" will be possible.  In that case, the code can be further simplified by eliminating the retval variable entirely from the function.
 
==== Example 1 : Remove List and retval declaration (colloc() from colloc.cc) ====
{|
{|
!Before !! After
!Before !! After
Line 22: Line 46:
|}
|}


==== Example 2 : Multiple Conditional (fopen() from file-io.cc) ====
==== Example 2 : ovl() extending over multiple lines ====
{|
{|
!Before !! After
!Before !! After
Line 55: Line 79:
== Detailed Instructions ==
== 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
The list of files which contain a possible instance where ovl() could be used 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


<pre>
<pre>
Line 71: Line 95:
Total: 61
Total: 61


Fixed: 0
Fixed: 61


{| class="wikitable"
{| class="wikitable"
Line 77: Line 101:


|-
|-
| ??? || corefcn/__dsearchn__.cc
| Jose || <strike> corefcn/__dsearchn__.cc </strike>
|-
|-
| ??? || corefcn/__ilu__.cc
| Rik || <strike> corefcn/__ilu__.cc </strike>
|-
|-
| ??? || corefcn/__qp__.cc
| Rik || <strike> corefcn/__qp__.cc </strike>
|-
|-
| ??? || corefcn/balance.cc
| Jose || <strike> corefcn/balance.cc </strike>
|-
|-
| ??? || corefcn/besselj.cc
| Rik || <strike> corefcn/besselj.cc </strike>
|-
|-
| ??? || corefcn/colloc.cc
| Jose || <strike> corefcn/colloc.cc </strike>
|-
|-
| ??? || corefcn/daspk.cc
| Rik || <strike> corefcn/daspk.cc </strike>
|-
|-
| ??? || corefcn/dasrt.cc
| Rik || <strike> corefcn/dasrt.cc </strike>
|-
|-
| ??? || corefcn/dassl.cc
| Rik || <strike> corefcn/dassl.cc </strike>
|-
|-
| ??? || corefcn/data.cc
| Jose || <strike> corefcn/data.cc </strike>
|-
|-
| ??? || corefcn/debug.cc
| Jose || <strike> corefcn/debug.cc </strike>
|-
|-
| ??? || corefcn/det.cc
| Rik || <strike> corefcn/det.cc </strike>
|-
|-
| ??? || corefcn/dirfns.cc
| Jose || <strike> corefcn/dirfns.cc </strike>
|-
|-
| ??? || corefcn/eig.cc
| Jose || <strike> corefcn/eig.cc </strike>
|-
|-
| ??? || corefcn/ellipj.cc
| Rik || <strike> corefcn/ellipj.cc </strike>
|-
|-
| ??? || corefcn/file-io.cc
| Jose || <strike> corefcn/file-io.cc </strike>
|-
|-
| ??? || corefcn/filter.cc
| Rik || <strike> corefcn/filter.cc </strike>
|-
|-
| ??? || corefcn/find.cc
| Rik || <strike> corefcn/find.cc </strike>
|-
|-
| ??? || corefcn/getgrent.cc
| Jose || <strike> corefcn/getgrent.cc </strike>
|-
|-
| ??? || corefcn/getpwent.cc
| Jose || <strike> corefcn/getpwent.cc </strike>
|-
|-
| ??? || corefcn/givens.cc
| Jose || <strike> corefcn/givens.cc </strike>
|-
|-
| ??? || corefcn/graphics.cc
| Rik || <strike> corefcn/graphics.cc </strike>
|-
|-
| ??? || corefcn/help.cc
| Jose || <strike> corefcn/help.cc </strike>
|-
|-
| ??? || corefcn/hess.cc
| Jose || <strike> corefcn/hess.cc </strike>
|-
|-
| ??? || corefcn/inv.cc
| Rik || <strike> corefcn/inv.cc </strike>
|-
|-
| ??? || corefcn/lsode.cc
| Rik || <strike> corefcn/lsode.cc </strike>
|-
|-
| ??? || corefcn/lu.cc
| Rik || <strike> corefcn/lu.cc </strike>
|-
|-
| ??? || corefcn/luinc.cc
| Rik || <strike> corefcn/luinc.cc </strike>
|-
|-
| ??? || corefcn/max.cc
| Rik || <strike> corefcn/max.cc </strike>
|-
|-
| ??? || corefcn/mgorth.cc
| Jose || <strike> corefcn/mgorth.cc </strike>
|-
|-
| ??? || corefcn/oct-stream.cc
| Rik || <strike> corefcn/oct-stream.cc </strike>
|-
|-
| ??? || corefcn/octave-link.cc
| Rik || <strike> corefcn/octave-link.cc </strike>
|-
|-
| ??? || corefcn/ordschur.cc
| Rik || <strike> corefcn/ordschur.cc </strike>
|-
|-
| ??? || corefcn/profiler.cc
| Rik || <strike> corefcn/profiler.cc </strike>
|-
|-
| ??? || corefcn/quad.cc
| Jose || <strike> corefcn/quad.cc </strike>
|-
|-
| ??? || corefcn/quadcc.cc
| Rik || <strike> corefcn/quadcc.cc </strike>
|-
|-
| ??? || corefcn/qz.cc
| Rik || <strike> corefcn/qz.cc </strike>
|-
|-
| ??? || corefcn/regexp.cc
| Rik || <strike> corefcn/regexp.cc </strike>
|-
|-
| ??? || corefcn/schur.cc
| Rik || <strike> corefcn/schur.cc </strike>
|-
|-
| ??? || corefcn/spparms.cc
| Rik || <strike> corefcn/spparms.cc </strike>
|-
|-
| ??? || corefcn/sqrtm.cc
| Rik || <strike> corefcn/sqrtm.cc </strike>
|-
|-
| ??? || corefcn/svd.cc
| Rik || <strike> corefcn/svd.cc </strike>
|-
|-
| ??? || corefcn/symtab.cc
| Rik || <strike> corefcn/symtab.cc </strike>
|-
|-
| ??? || corefcn/syscalls.cc
| Rik || <strike> corefcn/syscalls.cc </strike>
|-
|-
| ??? || corefcn/time.cc
| Rik || <strike> corefcn/time.cc </strike>
|-
|-
| ??? || corefcn/toplev.cc
| Rik || <strike> corefcn/toplev.cc </strike>
|-
|-
| ??? || corefcn/urlwrite.cc
| Rik || <strike> corefcn/urlwrite.cc </strike>
|-
|-
| ??? || dldfcn/__eigs__.cc
| Rik || <strike> dldfcn/__eigs__.cc </strike>
|-
|-
| ??? || dldfcn/__fltk_uigetfile__.cc
| Andy || <strike> dldfcn/__fltk_uigetfile__.cc </strike>
|-
|-
| ??? || dldfcn/__glpk__.cc
| Rik || <strike> dldfcn/__glpk__.cc </strike>
|-
|-
| ??? || dldfcn/__magick_read__.cc
| Rik || <strike> dldfcn/__magick_read__.cc </strike>
|-
|-
| ??? || dldfcn/__voronoi__.cc
| Rik || <strike> dldfcn/__voronoi__.cc </strike>
|-
|-
| ??? || dldfcn/amd.cc
| Rik || <strike> dldfcn/amd.cc </strike>
|-
|-
| ??? || dldfcn/audioread.cc
| Rik || <strike> dldfcn/audioread.cc </strike>
|-
|-
| ??? || dldfcn/ccolamd.cc
| Rik || <strike> dldfcn/ccolamd.cc </strike>
|-
|-
| ??? || dldfcn/chol.cc
| Rik || <strike> dldfcn/chol.cc </strike>
|-
|-
| ??? || dldfcn/colamd.cc
| Rik || <strike> dldfcn/colamd.cc </strike>
|-
|-
| ??? || dldfcn/convhulln.cc
| Rik || <strike> dldfcn/convhulln.cc </strike>
|-
|-
| ??? || dldfcn/dmperm.cc
| Rik || <strike> dldfcn/dmperm.cc </strike>
|-
|-
| ??? || dldfcn/qr.cc
| Rik || <strike> dldfcn/qr.cc </strike>
|-
|-
| ??? || dldfcn/symbfact.cc
| Rik || <strike> dldfcn/symbfact.cc </strike>
|-
|-


Line 553: Line 577:
dldfcn/__eigs__.cc:521
dldfcn/__eigs__.cc:521
dldfcn/__eigs__.cc:567
dldfcn/__eigs__.cc:567
dldfcn/__fltk_uigetfile__.cc:129
dldfcn/__fltk_uigetfile__.cc:129 Andy: I think this is a case where using ovl makes no sense
dldfcn/__fltk_uigetfile__.cc:131
dldfcn/__fltk_uigetfile__.cc:131 Andy: I think this is a case where using ovl makes no sense
dldfcn/__glpk__.cc:632
dldfcn/__glpk__.cc:632
dldfcn/__magick_read__.cc:279
dldfcn/__magick_read__.cc:279
Rik
1,072

edits

Retrieved from "https://wiki.octave.org/Special:MobileDiff/6973...7056"

Navigation menu