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

 
 

patch #8417: Consolidate large conditional type decisions

Submitted by:  David Spies <dspyz>
Submitted on:  Mon 24 Mar 2014 02:55:01 AM UTC  
 
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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>
Project 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 File(s):
   
   
Comment:
   

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

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by dspyz (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 25 Apr 2014 08:15:29 PM UTCdspyzAttached File-=>Added dispatch.diff, #31250
    Mon 24 Mar 2014 02:55:02 AM UTCdspyzAttached File-=>Added consolidate_dispatcher.diff, #31016

    Back to the top


    Powered by Savane 3.1-cleanup