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!
On Mon, Jul 16, 2012 at 11:50 PM, Christopher Sean Morrison
> 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.
> 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