views:

56

answers:

2

This is quite a common problem I run into. Let's hear your solutions. I'm going to use an Employee-managing application as an example:-

We've got some entity classes, some of which implement a particular interface.

public interface IEmployee { ... }
public interface IRecievesBonus { int Amount { get; } }
public class Manager : IEmployee, IRecievesBonus { ... }
public class Grunt : IEmployee /* This company sucks! */ { ... }

We've got a collection of Employees that we can iterate over. We need to grab all the objects that implement IRecievesBonus and pay the bonus.

The naive implementation goes something along the lines of:-

foreach(Employee employee in employees)
{
  IRecievesBonus bonusReciever = employee as IRecievesBonus;
  if(bonusReciever != null)
  {
    PayBonus(bonusReciever);
  }
}

or alternately in C#:-

foreach(IRecievesBonus bonusReciever in employees.OfType<IRecievesBonus>())
{
  PayBonus(bonusReciever);
}
  • We cannot modify the IEmployee interface to include details of the child type as we don't want to pollute the super-type with details that only the sub-type cares about.
  • We do not have an existing collection of only the subtype.
  • We cannot use the Visitor pattern because the element types are not stable. Also, we might have a type which implements both IRecievesBonus and IDrinksTea. Its Accept method would contain an ambiguous call to visitor.Visit(this).

Often we're forced down this route because we can't modify the super-type, nor the collection e.g. in .NET we may need to find all the Buttons on this Form via the child Controls collection. We may need to do something to the child types that depends on some aspect of the child type (e.g. the bonus amount in the example above).

Strikes me as odd that there isn't an "accepted" way to do this, given how often it comes up.

1) Is the type conversion worth avoiding?

2) Are there any alternatives I haven't thought of?

EDIT

Péter Török suggests composing Employee and pushing the type conversion further down the object tree:-

public interface IEmployee
{
  public IList<IEmployeeProperty> Properties { get; }
}

public interface IEmployeeProperty { ... }

public class DrinksTeaProperty : IEmployeeProperty
{
  int Sugars { get; set; }
  bool Milk { get; set; }
}

foreach (IEmployee employee in employees)
{
  foreach (IEmployeeProperty property in employee.Propeties)
  {
    // Handle duplicate properties if you need to.
    // Since this is just an example,  we'll just
    // let the greedy ones have two cups of tea.
    DrinksTeaProperty tea = property as DrinksTeaProperty;
    if (tea != null)
    {
      MakeTea(tea.Sugers, tea.Milk);
    }
  }
}

In this example it's definitely worth pushing these traits out of the Employee type - particularly because some managers might drink tea and some might not - but we still have the same underlying problem of the type conversion.

Is it the case that it's "ok" so long as we do it at the right level? Or are we just moving the problem around?

The holy grail would be a variant on the Visitor pattern where:-

  • You can add element members without modifying all the visitors
    • Visitors should only visit types they're interested in visiting
  • The visitor can visit the member based on an interface type
    • Elements might implement multiple interfaces which are visited by different visitors
  • Doesn't involve casting or reflection

but I appreciate that's probably unrealistic.

+5  A: 

I would definitely try to resolve this with composition instead of inheritance, by associating the needed properties/traits to Employee, instead of subclassing it.

I can give an example partly in Java, I think it's close enough to your language (C#) to be useful.

public enum EmployeeProperty {
  RECEIVES_BONUS,
  DRINKS_TEA,
  ...
}

public class Employee {
  Set<EmployeeProperty> properties;
  // methods to add/remove/query properties
  ... 
}

And the modified loop would look like this:

foreach(Employee employee in employees) {
  if (employee.getProperties().contains(EmployeeProperty.RECEIVES_BONUS)) {
    PayBonus(employee);
  }
}

This solution is much more flexible than subclassing:

  • it can trivially handle any combination of employee properties, while with subclassing you would experience a combinatorial explosion of subclasses as the number of properties grow,
  • it trivially allows you to change Employee properties runtime, while with subclassing this would require changing the concrete class of your object!

In Java, enums can have properties or (even virtual) methods themselves - I don't know whether this is possible in C#, but in the worst case, if you need more complex properties, you can implement them with a class hierarchy. (Even in this case, you are not back to square one, since you have an extra level of indirection which gives you the flexibility described above.)

Update

You are right that in the most general case (discussed in the last sentence above) the type conversion problem is not resolved, just pushed one level down on the object graph.

In general, I don't know a really satisfying solution to this problem. The typical way to handle it is using polymorphism: pull up the common interface and manipulate the objects via that, thus eliminating the need for downcasts. However, in cases when the objects in question do not have a common interface, what to do? It may help to realize that in these cases the design does not reflect reality well: practically, we created a marker interface solely to enable us to put a bunch of distinct objects into a common collection, but there is no semantical relationship between the objects.

So I believe in these cases the awkwardness of downcasts is a signal that there may be a deeper problem with our design.

Péter Török
Can you give an example?
Iain Galloway
@Iain, I just did, see my update :-)
Péter Török
Thanks. SO is a bit difficult to have an ongoing discussion on because the comments are a bit cramped. a) Why do you think this is better than the type switch? b) What if PayBonus needs something more than just a flag, e.g. if it needs the amount of the bonus (which only the child class would have)? - updated the question.
Iain Galloway
@Iain, again, I think I answered your questions while you were typing - see my update :-)
Péter Török
I'll update the question to reflect that answer!
Iain Galloway
A: 

You could implement a custom iterator that only iterates over the IRecievesBonus types.

PigBen
Presumably the iterator would need to do the type conversion inside it? Are you comfortable with that?
Iain Galloway
There's no getting around that. The point is that once you wrote your custom iterator, you wouldn't have to worry type conversion anymore. And yes, I'm perfectly comfortable with that.
PigBen