Let's think about this for a second. I'm sure you don't only have 2 sublcasses, so let's generalize this.
First things that come to mind are code duplication, extensibility and closeness. Let's expand on these:
Should you want to add more classes, you should change code in the least places possible.
Because the intersect
operation is commutative, the code for intersecting A
and B
should be in the same place as the code for intersecting B
and A
, so keeping the logic inside the classes themselves is out of the question.
Also, adding a new class shouldn't mean you have to modify existing classes, but rather extend a delegate class (yes, we're going into patterns here).
This is your current structure, I assume (or similar, probably a return type for intersect
, but not important for now):
struct Primitive
{
virtual void intersect(Primitive* other) = 0;
};
struct Sphere : Primitive
{
virtual void intersect(Primitive* other)
};
struct Plane : Primitive
{
virtual void intersect(Primitive* other);
};
We already decided we don't want the intersection logic inside Plane
or Sphere
, so we create a new class
:
struct Intersect
{
static void intersect(const Sphere&, const Plane&);
//this again with the parameters inversed, which just takes this
static void intersect(const Sphere&, const Sphere&);
static void intersect(const Plane&, const Plane&);
};
This is the class where you'll be adding the new functions, and the new logic. For example, if you decide to add a Line
class, you just add the methods intersec(const Line&,...)
.
Remember, when adding a new class, we don't want to change existing code. So we can't check the type inside your intersect functions.
We can create a behavior class for this (strategy pattern), which behaves differently depending on type, and we can extend afterwards:
struct IntersectBehavior
{
Primitive* object;
virtual void doIntersect(Primitive* other) = 0;
};
struct SphereIntersectBehavior : IntersectBehavior
{
virtual void doIntersect(Primitive* other)
{
//we already know object is a Sphere
Sphere& obj1 = (Sphere&)*object;
if ( dynamic_cast<Sphere*>(other) )
return Intersect::intersect(obj1, (Sphere&) *other);
if ( dynamic_cast<Plane*>(other) )
return Intersect::intersect(obj1, (Plane&) *other);
//finally, if no conditions were met, call intersect on other
return other->intersect(object);
}
};
And in our original methods we'd have:
struct Sphere : Primitive
{
virtual void intersect(Primitive* other)
{
SphereIntersectBehavior intersectBehavior;
return intersectBehavior.doIntersect(other);
}
};
An even cleaner design would be implementing a factory, to abstract out the actual types of the behavior:
struct Sphere : Primitive
{
virtual void intersect(Primitive* other)
{
IntersectBehavior* intersectBehavior = BehaviorFactory::getBehavior(this);
return intersectBehavior.doIntersect(other);
}
};
and you wouldn't even need intersect
to be virtual, because it would just do this for every class.
If you follow this design
- no need to modify existing code when adding new classes
- have the implementations in a single place
- extend only
IntersectBehavior
for each new type
- provide implementations in the
Intersect
class for new types
And I bet this could be perfected even further.