Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C#: Avoiding if (x is Foo) {...} else if (x is Bar) {...} for data structures

I have a family of data structures like:

abstract class Base {...}
class Foo : Base {...}
class Bar : Base {...}

and a method which takes a Base and converts it depending on which subclass it is:

void Convert(Base b) {
  if (b is Foo) 
    // Do the Foo conversion
  else if (b is Bar) 
    // Do the Bar conversion
...

Obviously this is terrible object orientation - the Convert method has to know about every derived class of Base and has to be changed every time that Base is extended. The 'normal' OO way of solving this problem is to make each derived class of Base responsible for converting itself, e.g.

abstract class Base {
  abstract Converted Convert();
...}
class Foo : Base {
  override Converted Convert(){...}
...}
class Bar : Base {
  override Converted Convert(){...}
...}

However, in the code that I am writing Base is a pure data structure (only getters and setters - no logic), and it is in another assembly which I do not have permission to change. Is there a way of better structuring the code that does not force the derived classes of Base to have logic?

Thanks

like image 276
Jonny Avatar asked Nov 13 '13 16:11

Jonny


2 Answers

If it hurts when you do that then don't do that. The fundamental problem is:

I have a method which takes a Base...

Since that is the fundamental problem, remove it. Get rid of that method entirely because it is awful.

Replace it with:

void Convert(Foo foo) {
   // Do the Foo conversion
}
void Convert(Bar bar) {
   // Do the Bar conversion
}

Now no method has to lie and say that it can convert any Base when in fact it cannot.

like image 94
Eric Lippert Avatar answered Oct 26 '22 03:10

Eric Lippert


I ran into a similar situation to this, and came up with a solution that's similar to pattern matching in a functional language. Here is the syntax:

Note that within the lambdas, foo and bar are strongly typed as their respectives types; no need to cast.

var convertedValue = new TypeSwitch<Base, string>(r)
            .ForType<Foo>(foo => /* do foo conversion */)
            .ForType<Bar>(bar => /* do bar conversion */)
        ).GetValue();

And here is the implementation of the TypeSwitch class:

public class TypeSwitch<T, TResult>
{
    bool matched;
    T value;
    TResult result;

    public TypeSwitch(T value)
    {
        this.value = value;
    }

    public TypeSwitch<T, TResult> ForType<TSpecific>(Func<TSpecific, TResult> caseFunc) where TSpecific : T
    {
        if (value is TSpecific)
        {
            matched = true;
            result = caseFunc((TSpecific)value);
        }
        return this;
    }

    public TResult GetValue()
    {
        if (!matched)
        {
            throw new InvalidCastException("No case matched");
        }
        return result;
    }
}

I'm sure it can be cleaned up, and in most cases Eric Lippert is right that this premise is fundamentally flawed. However, if you run into a case where your only other choice is a bunch of is and casts, I think this is a little cleaner!

like image 33
John Gibb Avatar answered Oct 26 '22 03:10

John Gibb