Theory
One should implement a common interface when:
Objects are similar and have a similar logic, and:
The shared logic is used in other places where the distinction between the types is not required.
For example, one shouldn't implement a common interface for both Dog: Animal and RSS, just because both have a common method Feed. Not only the logic behind those two methods is different (new Dog(...).Feed() would launch a robot which will give some food to a dog, while new RSS(...).Feed() will simply download the most recent RSS from the server), but you can hardly imagine a common Feeder which would be responsible of controlling dog food and downloading the last RSS feeds.
On the other hand, it would be convenient to have a common interface for both new Dog(...).Feed() and new Goldfish(...).Feed(), because while the implementation would be different, since the robot is expected to not putting a bowl of meat into an aquarium, the common interface still allows for the callers to not make any difference between the animals:
var myPets = new [] { (IAnimal)new Dog(...), new Goldfish(...) };
...
new FeedingEngine().FeedPets(myPets);
public class FeedingEngine
{
public void FeedPets(IEnumerable<IAnimal> pets)
{
foreach (var pet in pets)
{
if (pet.Status == AnimalStatus.Alive && pet.IsPotentiallyHungry)
{
pet.Feed();
}
}
}
}
Your case
Both of your proposals are correct. From the point of view of the consumer of ICollisionDetector, all I need to know is that I can pass any pair of spacial objects and get the collision result, no matter how.
There is a slight difference between those approaches: it's the fact that the first proposal requires you to double-check if all objects inheriting ISpatial are handled correctly by the collision detector. It can be error prone later: if, working on your code later, another developer implements another spatial object, let's say Sphere: ISpatial, but forgets to implement the collision logic, Collides(sphere, box) will fail, and it would be difficult to understand why is it failing, especially if the collision detector is used indirectly, like through the game engine.
The second approach is an opt-in. If the collision detector doesn't have a Collides(Sphere s, Box b), nobody would be able to call such a conversion. This being said, this itself creates another problem: you will not be able to use generics or any high-level abstraction. For example:
var box = new Box(...);
var torus = new Torus(...);
var engine = new GameEngine();
engine.Show(new [] { box, torus });
engine.StartColliding(); // Moves objects to the center of the map and stops on collision.
is easy to implement with your first proposal if both Box and Torus implement ISpatial, but would be extremely difficult to do with your second proposal.
In order to be able to use your first proposal while reducing the issue I noted above, here's a third proposal:
public interface ISpatial
{
bool IsColliding(ISpatial obj);
}
public class CollisionDetector
{
public bool IsColliding(ISpatial s1, ISpatial s2)
{
return s1.IsColliding(s2);
}
}
public class Torus : ISpatial
{
public bool IsColliding(ISpatial obj)
{
// Check that obj is not null.
// Do the collision logic for known types.
var box = obj as Box;
if (box != null)
{
// Do the collision logic.
return result;
}
return obj.IsColliding(this);
}
}
public class Cube : ISpatial
{
public bool IsColliding(ISpatial obj)
{
// Check that obj is not null.
// Do the collision logic for known types.
return obj.IsColliding(this);
}
}
Since the cube class is created first, it is unaware of torus; instead of throwing a NotImplementedException, it simply delegates the collision detection logic to the torus, which is well aware of the cube object, since the torus class was the second one to be created.
Note: of course, you still have to handle the situations where neither the first, nor the second object are aware of each other (for example if two objects were created by two different developers at the same moment of time). Actual code above will simply overflow and crash, but you can easily change it to throw NotImplementedException instead.
