I would like to work on this task.

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

The work on this task is ready to be reviewed.

Can you explain what your first loop is doing?  "transferring data" doesn't really seem to convey much behind the intent nor address several peculiarities in that loop... :)

A few pragmatic issues:

• shouldn't use // comments -- did you disable strict mode?  the build should be reporting those as problems to you.  use /* comment */


• mixed tabs and spaces for indentation.  you used only spaces.  We use hybrid spacing where it's 4spaces, then 1 tab, the 1tab and 4 spaces, then 2 tabs, etc.  See our HACKING file or other sections of the file you're editing for examples.  We provide emacs local variables and vim modelines to format it automatically with those editor environments, but you may have to set your environment up manually (tabstop=8, indent=4)


• braces should be on same line -- you have a bunch of if statements near the end


• lenghts is spelled wrong


• what's the purpose of INTCLAMP() in the first loop?


• why aren't you using bn_area_of_triangle() ? :)  You should reuse existing API as much as possible (see include/bn.h and other headers)

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.

The work on this task is ready to be reviewed.

So, I hope, my code is now right intended and I implemented your hints right. The "transferring data" section has been reorganized, but is doing in fact the same as earlier, namely bringing the different vertices from the rt_bot_internal, into an logically structured array, which makes it much easier to work with them . My intention was to make the following operations( computing the lengths of the edges) more comprehensible and so improve the maintainability, as it is then not necessary to get the vertices complicated out of the rt_bot_internal, each time you need one point.

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.

That is better but you didn't address all of the points I raised.  Please address all comments and questions. :)

Also noticed that you removed the vague comment altogether.  If it needs to be explained, and it does, then you should explain it with a comment block in the code.  Note also that I intentionally mentioned /* comment */ and not /*comment*/ as that is covered by our style guide.

Please check over the list again and cover all the points identified as all of them are issues of concern.

• The INTCLAMPs had no deeper use, they were just part of the old function, I copied this part from. I removed them now to make the code not to big und complex


• Thanks for the /include/bn hint, beside the triangle function, I could also use a distance function from this header.


• Due to this, the old use for transferring the vertices is'nt anymore, but it came to my mind, that I need this array also in the part of the function, where I search for the exterior edges. These are nearly 20 if-clauses and if I would get for each the right vertex out of the rt_bot_internal, this would make the code much more complex, then this array does now.


• I hope I have now enough comments, to make the things, I'm doing comprehensible, if it's not so, please tell me.

The work on this task is ready to be reviewed.

Very nice work Johannes.  We'll likely have a follow-on task to enable this new function and properly test it, but the implementation looks pretty good now.  We'll be sure to credit you in our authorship notes on this one.

I only noticed a couple issues, but they're minor in the scope of effort you've put in on the patch.  I noticed was the assumption that "default" is one of the RT_BOT_PLATE* modes, which won't necessarily always be true.  The code should be explicit and default to same as RT_BOT_SOLID. The other issue I noticed is a curious lack of += *= etc operators.  It's clearly called for in the accumulation of whole_bot_overall_area, for example, and a few other places.

Still, in all very nice use of the bn functions and hopefully validation will show that you got everything right! :)

Congratulations, this task has been completed successfully.

As GCI comes to a close, we wanted to take the time to say THANK YOU for all your efforts.  This comment interface closes after GCI is over, so you're encouraged to join our mailing list where we'll be announcing contributions from GCI participants like yourelf over the upcoming months: 

https://lists.sourceforge.net/lists/listinfo/brlcad-news

If you've provided your full name, we'll be sure to credit you in our authorship documentation and you'll see your name in a future announcement.  If you contact us at devs@brlcad.org or via IRC, we'll even let you know when your work is integrated and follow up with updates.  You're welcome and encouraged to contact us any time, especially if you have a question about how to continue participating in Open Source after GCI is over, but even if just to keep in touch.  Note that ongoing participation in Open Source is one of the most impressive skills to have on your resumé.  Take care, be well, and thank you again!