The Law of Demeter

I’ve been refactoring big pieces of legacy code recently and my work would be much easier if authors would have followed the Law of Demeter. It’s not even LoD per-se, it’s mutated version, because I’m not talking about calling methods, rather accessing fields. The general guideline is – where possible, function should only operate on a minimal required subset of data. Consider the following code snippet:

float GetDistanceFromCamera(const Object& obj, const Camera& camera)
    return (obj.GetPosition() - camera.GetPosition()).Length();
On the first sight, it looks reasonable, small function performing well-defined operation. It’s not really future-proof, though. Imagine we’d like to use it in multi-threaded environment, when multiple threads can read/write object/camera positions. One standard way to overcome this, is double-buffering. Sure, we can buffer whole objects, but it would be easier to only buffer attributes. Problem is, our GetDistanceFromCamera take object/camera as argument, even if it doesn’t really need 90% of their data, it’s only interested in their positions. In such case, we’d be much better with function refactored to:
float GetDistanceFromCamera(const Vec3& objPos, const Vec3& camPos)
    return (objPos - camPos).Length();
Of course, example is a little artificial as now it doesn’t even make much sense in having separate function here, we can easily write it as:
float GetDistanceBetweenPoints(const Vec3& p1, const Vec3& p2)
and use it in other places, not only for camera/object distance.

Old comments

Paul Evans 2009-02-15 19:12:11

I don’t think it is that artificial because I have seen code very much like your first example.
Nice article.

getposition 2010-04-03 10:28:35

[…] … Unfortunately at the moment the getPosition command for the Primitive Object is broken. …The Law of Demeter | .mischief.mayhem.soap.I've been refactoring big pieces of legacy code recently and my work would be much easier if authors […]