patchGNU Octave - Patches: patch #8417, Consolidate large conditional...

 
 

patch #8417: Consolidate large conditional type decisions

Submitter:  David Spies <dspyz>
Submitted:  Mon 24 Mar 2014 02:55:01 AM UTC
   
 
Category:  Core : new feature Priority:  5 - Normal
Status:  Need Info Privacy:  Public
Assigned to:  None Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Wed 06 Jul 2016 12:39:50 PM UTC, comment #4: 

David, thanks for submitting a revised version of this patch.

JWE asked for some comments in the code.  If you are still interested in this patch, could you submit a version with comments for each of the functions?

Lachlan Andrew <lachlan>
Fri 25 Apr 2014 08:26:04 PM UTC, comment #3: 

Oh, also I don't think "find" should be added to octave_value_class.  This is a typical pattern that will necessarily persist for every function in Octave: find out actual type of object; pass it off to proper template instantiation of the actual function.

Unfortunately, C++ makes this hideously difficult to do cleanly (as with anything involving C++ templates.  I suppose you could also use macros... but then you'd have to use macros).  Adding all the functions to some giant monolithic class doesn't really help the problem because you still have to write a separate function-call for each type for each function.

The only way I could see to avoid that is using a template template function (by the way, don't take me as the sort of person who tries to solve everything with template templates.  This is the first time I've ever had to use one, and I hope the last).

David Spies <dspyz>
Fri 25 Apr 2014 08:15:29 PM UTC, comment #2: 

Unfortunately, I lost dispatch.h
I didn't realize I hadn't committed it and somehow I managed to delete it.  I'm proposing a bugfix which adds ignored files to .hgignore so this doesn't happen again: https://savannah.gnu.org/patch/index.php?8442

(with this, you can tell what you've changed just by doing 'hg status')

I just rewrote dispatch.h (the cases are adopted from the old find-function.  Any cases that are missing were missing in the original find-function)

Here's the patch again (with it this time)

(file #31250)

David Spies <dspyz>
Wed 23 Apr 2014 03:51:10 PM UTC, comment #1: 

I tried to build with this patch but dispatch.h is missing.

Looking at it, I think it would help me and future readers if there were some comments to explain the intent of the new templates.

We would also like to have the patch follow the Octave coding conventions.  If you need more explicit help with that let us know.  We'll also want a changelog entry

Now I'm wondering whether we should (or can) add a "find" function to the octave_value class, and then dispatch to the individual matrix classes so that this could be more easily extended to new matrix types in the future.  I.e., if you add a new matrix type, you can add the find functionality there instead of having to modify the find function.

John W. Eaton <jwe>
Group administrator
Mon 24 Mar 2014 02:55:01 AM UTC, original submission:  

While working on the "find" function, I was annoyed by the large "if-else-if" list of conditionals which all do exactly the same thing (check if the input matrix is a particular type and then call the "real" find function with the proper template type).  I had to go down the list of calls and add a fourth parameter to each and every one.  There should be utility "dispatcher" functions which take a functor template template and an octave_value_list of arguments and call the functor with the proper template argument.  Then calls such as the long conditional statement in "find" can be consolidated into one place rather than having to have them for every single function which can take multiple types.  And if a function call needs to be changed, it only needs to be changed in one place.

I've attached an example for how to do this with find (this also includes my changes from https://savannah.gnu.org/patch/?8386 sans the fourth parameter).  Now any other function which takes the same matrix types as "find" as an argument can call the same "dispatch_all" function to call the functor with the proper template argument.

For functions which take multiple arguments with variable types, this can be done by nesting the functors (kind of like currying.  If you'd like I can give an example).

David Spies <dspyz>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #31250:  dispatch.diff added by dspyz (30KiB - text/x-patch)
file #31016:  consolidate_dispatcher.diff added by dspyz (24KiB - text/x-patch)

 

Digest:
   patch dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Updated the item)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by dspyz (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-01-18 siko1056 Dependencies- Depends on patch #8386
    2020-01-18 siko1056 CategoryNone Core : new feature
        StatusNone Need Info
    2014-04-25 dspyz Attached File- Added dispatch.diff, #31250
    2014-03-24 dspyz Attached File- Added consolidate_dispatcher.diff, #31016

    Back to the top

    Powered by Savane 3.14-3b9d.
    Corresponding source code