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.