Thorsten Froehlich <tho### [at] trfde> wrote:
> You really don't want to fill a lightweight class like ray with data like
> that. The recursion level is a property of the algorithm, not a ray. As
> such, moving recursion counting into the ray violates separation of data and
> algorithm, which in the long term practically always turns out to bit you
> somewhere else later when you do not expect it.

I have some years of experience as a software developer, so I know about the
pitfalls of future changes - or future maintenance for that matter. And that is
exactly the reason why I say it is a very bad idea the way it is now.

To put it on a more academic level: The choice to consider the trace level a
part of the "Trace" algorithm instead of the "Ray" data to work on has forced
an unjustifiedly close coupling between the "Trace" and "RadiosityFunction"
objects.

In fact, the trace level is part of multiple algorithms; it already shows in
your "Trace" interface: The trace level is the only changing data that is
exposed to callers. Why? Because it shouldn't be in there in the first place.
At least the "RadiosityFunction" algorithm needs it, too.

So instead of just passing it along with the ray as it is passed on from
algorithm to algorithm (e.g. from "Trace" to "RadiosityFunction" and back), the
current architecture requires that "RadiosityFunciton" must only be called by
the very same instance of "Trace" that would in turn be called by the
"RadiosityFunction" instance - and vice versa! If that isn't strong coupling, I
have never seen any. And why? All because of the "age" of the ray!

Yes, I *do* want to fill a lightweight class like ray with data like this.
Because it is inherently connected to a certain ray being processed. Just like
the information whether it is a primary ray, a radiosity ray, a photon ray, or
whatever.

If you want to keep the "Ray" class lightweight, fine - but then define a
separate kind of "companion" data object for this type of information (it could
then also contain all those flag currently in "Ray"). Misusing the "Trace"
object as a container for this information is a bad idea - and not passing it
as a parameter, but instead relying that a certain function be only called by a
certain "Trace" object, is a *very* bad one.

Post a reply to this message