Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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

share|improve this question
    
You've touched on some solutions, but the answer for you really depends on why you need to ensure that it is a ImyBaseDeviation instead of a ImyBase. Are there other implementations of ImyBase that can't be converted to the derived class? Are there things on the derived interface that can be pulled up to the parent? –  Bobson May 1 at 0:36
    
@Bobson basically the deviation of ImyBase generally implement ImyBase and some other interface(s) –  im_a_noob May 1 at 0:45
    
Do all of the implementations of the interface with the void Add(ImyBase) method accept specifically ImyBaseDeviation, or do they each accept different concrete types? –  cbojar May 1 at 5:02
    
I think this is simply case of incorrect design. And we can't help you with that unless we know what each interface and Add methods really do. Or are supposed to do. –  Euphoric May 1 at 6:57
    
@cbojar they accept different concrete types –  im_a_noob May 1 at 11:43

2 Answers 2

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.

share|improve this answer
    
This You should be able to find a solution that uses the type system instead of a manual check. is surely what I'm trying to archieve. I will have a closer look at your hints to find the best solution in my case. –  im_a_noob May 1 at 13:17

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.