0

Given the following class structure:

class Base {}
class DerivedA: Base {}
class DerivedB: Base {}
class Container
{
    public List<Base> Items { get; }
}

where the list of Derived objects grows over time as my application develops more functionality. I'm trying to do:

Container container = ReturnsContainer();
foreach (var item in container.Items)
{
    doStuff(item);
}

where doStuff(item) is overloaded by derived type.

void doStuff(DerivedA) {}
void doStuff(DerivedB) {}

That doesn't work as the compiler says "cannot convert from 'Base' to 'DerivedA'" and "The best overloaded method match doStuff(DerivedA) has some invalid arguments". The best approach I can come up with is:

Container container = ReturnsContainer();
foreach (var item in container.Items)
{
    if (item is DerivedA)
    {
        doStuff((DerivedA)item);
    }
    else if (item is DerivedB)
    {
        doStuff((DerivedB)item);
    }
}

Any thoughts on how I can make this cleaner? Since my Derived types will only grow over time, I would have to go back and add to this long if structure to continue to make this work.

I think the container.Items.OfType<> solution would translate to a growing (and unnecessary) performance hit as the number of Derived types increases.

Do I have to use generic delegates for this? Seems like an overly complex solution for something that feels like it should be simple polymorphism.

Edit: For further clarity, let's say the Base and Derived type hierarchy is on a separate API layer and is not modifiable (final classes). The solution has to work with the existing inheritance structure and cannot extend the Base and Derived objects. Though the API layer can grow new Derived classes as time goes on.

Jeremy
  • 1,168
  • 3
  • 17
  • 31
  • 1
    Its *far* from simple polymorphism. It has to do with covariance and contravariance (for which I always confuse the meaning of each). Try googling those 2 topics! – Jamiec Nov 28 '12 at 15:14
  • @Jamiec, that's right. It feels like polymorphism but isn't. I can't find an example that matches my situation exactly. A delegate approach seems to be the best fit (trying it now) but I'm trying to avoid making my code hard to read. – Jeremy Nov 28 '12 at 15:17
  • @Jeremy It's not polymorphism because you've inverted it. It seems kinda like polymorphism because that's what you should be using, you're just not. – Servy Nov 28 '12 at 15:25

4 Answers4

3

Either you will have to use run-time type information (e.g. reflecting over the type) to determine which function to call or you can use an abstract method (doStuff) declared on the base class and implemented in the derived classes. In many cases you would like to avoid a solution using reflection but if you cannot modify the classes involved you have no other option, and using dynamic as has been proposed in other answers is a very simple way of achieving what you want.

E.g. implement the two overloads:

void doStuff(DerivedA a) {
  ...
}

void doStuff(DerivedB b) {
  ...
}

And call them from the loop:

foreach (dynamic item in container.Items)
  doStuff(item);
Martin Liversage
  • 104,481
  • 22
  • 209
  • 256
  • The problem is I have limited access to the Base and Derived classes. They're on a different layer, and I'm looking for a solution that will allow me to leave the type structure unchanged. – Jeremy Nov 28 '12 at 15:18
  • If you cannot modify the classes you will at some point have to branch your code on the actual type of the item in the list. There are somewhat more elegant ways of doing that. For instance see this question: http://stackoverflow.com/questions/156467/switch-pattern-matching-idea – Martin Liversage Nov 28 '12 at 15:24
2

There are a couple of options here, but it really boils down to how can doStuuf know what to do if you're going to add new derived types in the future?

So, one option is if Container really does only contain 1 type at a time then you could make it generic:

class Container<T> where T: Base
{
    public List<T> Items { get; }
}

Now when you iterate it, you know that Items is a List of DerivedA or DerivedB

Container<DerivedA> container = ReturnsContainerOfDerivedA();
foreach (var item in container.Items)
{
    doStuff(item); // item is definately DerivedA
}

This would mean you either have multiple doStuff methods

public void doStuff(DerivedA item){...}
public void doStuff(DerivedB item){...}

or if more appropriate you can have 1 which takes the base

public void doStuff(Base item){...}

That is the advantage of simple polymorhism!


The second option is to define doStuff as taking a dynamic if you're using C#3.5

public void doSomething(dynamic item){..}

But I would personally avoid this option!

Jamiec
  • 133,658
  • 13
  • 134
  • 193
  • Tried this just now, however I was hoping to retain doStuff(DerivedA) and doStuff(DerivedB), each of which handles each Derived type differently. A dynamic object would mean that I'd have to segment my implementation of doStuff() via reflection, which would end up with the same amount of conditional branching as well. – Jeremy Nov 28 '12 at 15:25
  • @Jeremy - in case its not clear, the second option (using dynamic) is completely unconnected to the first half of this answer. If you go with a generic Container, you would need to have different `doStuff(DynamicA)` and `doStuff(DynamicB)` methods. See update. – Jamiec Nov 28 '12 at 15:26
  • I see. Looks like filtering by type is indeed the way to go. – Jeremy Nov 28 '12 at 15:29
  • @Jeremy - no, defining this behaviour on `Base` is the correct way to go. See answer by `@Servy`. Anything else you do is just a hack! – Jamiec Nov 28 '12 at 15:30
  • I'd love to hack this... My research tells me that there's something smelly about the API implementation (the final Base and Derived hierarchy), but there's little I can do about it for now. – Jeremy Nov 28 '12 at 15:34
  • Fair enough, but at least you understand the problem now. – Jamiec Nov 28 '12 at 15:35
1

The simplest solution (without changing Base/DerivedA etc.) is to cast your objects to dynamic:

foreach (dynamic item in container.Items)
{
    doStuff(item);
}

so the actual method to call is determined at runtime.

sloth
  • 99,095
  • 21
  • 171
  • 219
  • 3
    screw proper design, just use `dynamic`... – Servy Nov 28 '12 at 15:20
  • Actually `dynamic` has *nothing* do to with design. There are a lot of cases where the use `dynamic` makes your code simpler/easier to read. – sloth Nov 28 '12 at 15:41
  • 1
    Design has everything to do with using the proper tool for each type of task. `dynamic` is one tool. Determining whether `dynamic ` is the appropriate tool for the current job has *everything* to do with design. `dynamic` does indeed have a place; there are tasks for which it is the proper tool, this simply isn't one of them. `dynamic` is one of those things that *can* be used much more than it *should*m, and it is often used as a crutch for those unable or unwilling to use more appropriate tools. – Servy Nov 28 '12 at 15:45
  • I'd say this is the most straightforward answer to what the question calls for, but I can see why this is not the most beneficial solution design-wise. – Jeremy Nov 28 '12 at 15:49
  • @Servy Could you please elaborate how this case is not a parade example for the use of `dynamic`? It's not always logical to put every piece of code that somehow belongs to a class into that very class itself (and often, you outright can't). That's why other languages provides better tools: look at F# where you could just use a `match`, or Clojure, where you probably would just use protocols. – sloth Nov 28 '12 at 16:04
  • @DominicKexel So consider the case where a new derived type is created. Not only do you need to create the new type, and implement whatever behavior it currently has, but you need to find some other arbitrary class in an entirely separate API and create a new `doStuff` method taking the newly created type as a parameter. You then need to copy/paste one of the other implementations (in all probability, depending on what it does) and make some small modification based on the differences of the new type (whatever that difference is). Just *knowing* that you need to do that is a nightmare. – Servy Nov 28 '12 at 16:08
  • @DominicKexel The other side is simply being forced to implement an abstract method when first creating the type. – Servy Nov 28 '12 at 16:09
  • @Servy You could simply define `void doStuff(Base d) { // nothing or default behaviour}`. – sloth Nov 28 '12 at 17:03
  • @Servy The other side is the OP already said multiple times that changing `Base` is not an option. – sloth Nov 28 '12 at 17:03
1

The user of the Base object should need to know what type of Base object it is. In a case such as this it is pretty clear that Base should probably be abstract, and it should have an abstract method defining whatever behavior doStuff needs from a Base object. (If making Base abstract isn't an option you could have it define a virtual method with no body that's intended to be overridden, or you could have all of the derived objects implement an interface.)

Once you've done that each derived object can override the defined method to provide whatever behavior differs between each type.

Once you've done that Container doesn't need to cast/convert the object to anything other than Base, each Base will already have all of the information you need for a single doStuff method.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • The OP already mentioned that he can not change `Base` and the derived classes. – sloth Nov 28 '12 at 15:26
  • @DominicKexel Actually his exact words were, "I have limited access to the Base and Derived classes". It's not that he *can't* modify them, it would just be more work. I don't dispute that doing it correctly may be a bit more work, but it's still the correct way to solve the problem. – Servy Nov 28 '12 at 15:28
  • This is 100% the correct answer IMO – Jamiec Nov 28 '12 at 15:31
  • Actually he said "They're on a different layer", which could mean that they're some kind of DTO on the database layer, and the code in question is on the GUI or business layer for example. While it seems natural to alter `Base` as you described, it may be a bad idea from an architectural point of view. – sloth Nov 28 '12 at 15:33
  • Sorry, edited my question to express this as well. This answer is the approach I would have taken had the layer restrictions not been in place. – Jeremy Nov 28 '12 at 15:36
  • 1
    @Jeremy If you can't modify `Base` or any of the derived classes then make wrappers for them. Make, `Base2` `DerivedA2`, etc. that wraps the underlying classes and provides whatever additional functionality you need, (basically using the adapter pattern). Rather than just giving up entirely on having a proper hierarchy, change your problem to "how do I convert what I have to a proper hierarchy?" – Servy Nov 28 '12 at 15:39