Interface method signature not restrictive enough

Question

I wrote a piece of code at my job sometime ago. While writting the code I wrote few interfaces who would allow me to add more flexibility and at that time I was also trying to understand OOP concept more deeper. So in the interface I wrote I have a method signature similar to this :

void Add(ImyBase objToAdd)

And then I have few class implementing this interface. But what is bothering me is that I find myself doing this kind of thing pretty often in this Add method

public void Add(ImyBase objToAdd)
{
    if(!(objToAdd is ImyBaseDeviation))
        throw new ArgumentException("blablabla");

    //do stuff
}

At the time it was a smell to me but I didn't really bothered with it simply because I'm solo on this projet and this piece of code is only used within my compagny and there only 2 IT guys in there (me + some one else) and this is not near to change. Anyway I recently came back to that piece of code and from my previous reading (actually more than that but this one is pretty similar) it now actually really look bad to me and I'm trying to figure out the best way to improve this piece of code.

I don't like the code as it stand now because the interface allow more than the concrete type generally allow so basically the concret implementation doesn't respect the contract given by the interface.

I tough of a few solutions

First simply delete the Add method from the interface since anyway calling Add have a high chance to throw an exception. So to call Add you need the concret type anyway. So here the object would still have a Add method but with the accepted type as a parameter.

Secondly add a method who would look something like this in the interface

bool IsValidToAdd(ImyBase objToTest)

But that doesn't satisfy me.

So I'm seeking advise from more advance people like you to help me to figure out what would be the best way to implement this so it stay the cleanest possible.

Sorry I'm not a native anglophone so if my question need editing or grammar fixing please go ahead. Thank you

Answer 1

You're focusing on that one interface, but you have at least four different customization points available to you:

• The interface itself.
• The code that implements the interface.
• The code that calls Add.
• ImyBase and its derivatives.

You haven't provided enough information about the latter two, but that's likely where your solution is, since you've been unable to find a satisfactory solution using only the first two. Some possible solutions are:

• Look into using parameterized classes or methods (the ones with <Type>) to restrict the types while still avoiding duplication.
• Move some functionality from your concrete classes implementing Add to the ImyBase objects. Generally, container-like objects benefit from being as generic as possible, only doing container stuff and nothing else.
• Try moving Add to the ImyBase object. Sometimes reversing who gets added to whom simplifies the code, or at least helps open your eyes to other possibilities, which is why it is useful as an exercise.

You're on the right track with your criteria for a good design. You should be able to find a solution that uses the type system instead of a manual check.

Answer 2

It's not an easy question to answer without some examples, but I see three ways you can go, depending on the actual in-use scenarios.

1. Get rid of ImyBase. You might replace specific (or all) uses with ImyBaseDeviation, and strip it out of anywhere it wasn't used.
   Do this if: there's nothing that uses ImyBase which doesn't also require something in ImyBaseDeviation (like a second, different implementation of ImyBase, such as IAnimal -> ICat and IDog).
2. Pull things from ImyBaseDeviation back into ImyBase. You'll want to pull enough functionality up that everything clearly should require one or the other. Remember, you can use the less-specific interface anywhere you're expecting the more-specific one.
   Do this if: the two interfaces actually mean different things (such as IAnimal -> IHerbivore).

3. Do #2, but "shadow" the base interface's methods in the specific one. This happens a lot with generics. There might be a IFoo interface:

   interface IFoo
   {
     object GetValue();
   }
   
   interface IFoo<T> : IFoo
   {
     new T GetValue();
   }
   

   This is how IEnumerable<T> handles it, for example.
   Do this if: the two are very closely related, but one uses a more specific type than the other.

Hopefully this points you in the right direction, at least.