Sean,
I actually moved those functions and committed before I saw this e-mail. So
I might have unnecessarily moved those functions. I will make the suggested
changes, and remember your advice about the naming conventions :)
Thanks you for the detailed review of my code!
Regards,
On Mon, Jul 16, 2012 at 11:50 PM, Christopher Sean Morrison
<brl...@mac.com>wrote:
>
> Anurag,
>
> I was looking through the new voxelize command and had a few comments.
> It's looking like really good progress but we should make sure we're
> encapsulating across library boundaries so we don't end up with an ad hoc
> API. A few things I noticed:
>
> 1) The three helper/callback functions presently in libanalyze should be
> marked HIDDEN as they shouldn't be public API.
> 2) Of course, making them static means at least two of them as written
> wouldn't belong in libanalyze but, rather, in the caller in
> libged/voxelize.c; however before you move them ...
> 3) as an API, voxelize() taking an rtip seems like the wrong type to be
> using. I'd expect to be able to pass one or more rt_db_internal's. It's
> an implementation detail that it's shooting rays so most of that setup
> would move from voxelize.c to voxels.c in libanalyze.
> 4) Moving the ray trace calling to libanalyze means passing those
> callbacks makes less sense, too. You'd need a means to either pass in or
> otherwise have voxelize() return a voxel data set.
>
> General rule of thumb, if you're writing the function name in camelCase,
> it probably should be HIDDEN. ;) I have absolutely no problem with that
> convention, but it's not the naming convention those libraries use. It may
> be worthwhile to discuss exactly what the function signature in libanalyze
> should be before implementing it, so you don't spend time moving things
> around unnecessarily (e.g., my second comment above). With that "black
> box" interface defined, you can go to town on getting the implementation to
> do what is needed.
>
> Cheers!
> Sean
>
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> BRL-CAD Developer mailing list
> brlcad-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/brlcad-devel
>
--
Anurag Murty
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
BRL-CAD Developer mailing list
brlcad-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/brlcad-devel
