Design new API function to reduce duplication #3BRL-CAD
Status: ClosedTime to complete: 72 hrs Mentors: SeanTags: design, code, C, API

This task involves designing a new function that will measurably help reduce code duplication. You'll need to have already identified a piece of duplicated code that needs to be refactored and/or modified.

This task involves writing just the header file declaration and a doxygen comment that fully describes how the function behaves.  It is your responsibility to make sure that the return types and function arguments are well thought out, fit well with our existing conventions, and is a clean simple yet effective design.  Think it through.

Submit a patch file with your change, modifying the corresponding public include/header file that goes with the library being modified.

Note candidate libraries to modify are our core libraries:

  • libbu for basic utility functions
  • libbn for basic math and simple geometry functions
  • librt / libnmg / libbrep for more complicated geometry functions
  • libanalyze for analysis functions
  • libged for geometry editing (commands)
  • ... see src/README for a brief listing

 

Uploaded Work
File name/URLFile sizeDate submitted
bn_ccw_helper.patch5.0 KBJanuary 03 2014 18:47 UTC
bn_poly_sort.patch5.3 KBJanuary 03 2014 23:03 UTC
http://sourceforge.net/p/brlcad/code/59276/n/aJanuary 04 2014 11:29 UTC
Comments
Johannes Schulteon January 3 2014 18:01 UTCTask Claimed

I would like to work on this task.

Mandeep Kaur on January 3 2014 18:04 UTCTask Assigned

This task has been assigned to Johannes Schulte. You have 72 hours to complete this task, good luck!

Johannes Schulteon January 3 2014 18:47 UTCReady for review

The work on this task is ready to be reviewed.

Johannes Schulteon January 3 2014 18:51 UTC

I did implementation and design directly at once, because at the moment there are no implementation tasks. Nevertheless, if you want to have it structured, create one, and I will claim it.


This helper function was worked out and introduced by a GSoC student of yours: http://brlcad.org/wiki/User:Crdueck/log#28.2F06.2F2012: . While I don't fully understand the logic, I think it's based on this SO comment : http://stackoverflow.com/questions/6880899/sort-a-set-of-3-d-points-in-clockwise-counter-clockwise-order#comment8189696_6881093

Sean on January 3 2014 22:19 UTCnot digging it

Johannes, this doesn't feel like a good API addition as-is.  The new function fails to stand on it's own.  To make sense (as public API), the types would need to be point_t's and plane_t's, which is obviously problematic for bn_sort().  Even without doing that, the function description is misleading as it doesn't do any sorting or counter-clockwising intrinsically, it's merely a comparison function (used elsewhere to do ccw sorting).


I think what will make more sense is something like bn_polygon_sort() or something similar where that function does the bu_sort and the helper function can remain static. That way, you still get the code reduction and a public API call that makes sense.


Remember that the implementation of any public API *must* check all function parameters for validity.  They must assume the worst.  It should gracefully handle bad input (e.g., null pointers, negative sizes, etc).


 

Sean on January 3 2014 22:19 UTCTask Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Johannes Schulteon January 3 2014 23:03 UTCReady for review

The work on this task is ready to be reviewed.

Johannes Schulteon January 3 2014 23:05 UTC

As you may have seen, I implemented a volume function for arbn, so when this task is finished, a task for that would be great.

Sean on January 4 2014 07:45 UTCTask Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Sean on January 4 2014 07:49 UTCfew issues

Only public API should use the bn_ prefix.  The static function should be just sort_ccw() or something similar/simple.  Also, since it's in a library, it should be marked HIDDEN, not 'static'.


The rest looks good.  Assuming you can fix those two issues on commit (accidentally marked needing work).


 

Sean on January 4 2014 08:02 UTCa volume function for polyhedral solids (ARBN)

has been posted: http://www.google-melange.com/gci/task/view/google/gci2013/5782712895930368

Johannes Schulteon January 4 2014 11:29 UTCReady for review

The work on this task is ready to be reviewed.

Sean on January 4 2014 16:34 UTCTask Closed

Congratulations, this task has been completed successfully.