design pattern to remove multiple if/else clauses with related objects

I have inherited the following (terrible) code and am wondering how best to refactor it.

There are large if/else clauses all over the codebase, one of which is similar to below :

public class BaseResultItem
{
    public int Property1 { get; set; }
}

public class ResultItem1 : BaseResultItem
{
    public int Property2 { get; set; }
}

public class ResultItem2 : BaseResultItem
{
    public int Property3 { get; set; }
}

public class BaseHistoryItem
{
    public int Property1 { get; set; }
}

public class HistoryItem1 : BaseHistoryItem
{
    public int Property2 { get; set; }
}

public class HistoryItem2 : BaseHistoryItem
{
    public int Property3 { get; set; }
}

public class HistoryBuilder
{
    public BaseHistoryItem BuildHistory(BaseResultItem result)
    {
        BaseHistoryItem history = new BaseHistoryItem            
        {
            Property1 = result.Property1
        };

        if (result is ResultItem1)
        {
            ((HistoryItem1)history).Property2 = ((ResultItem1)result).Property2;
        }
        else if (result is ResultItem2)
        {
            ((HistoryItem2)history).Property3 = ((ResultItem2)result).Property3;
        }

        return history;
    }
}

Note that this is a simplified example and there are many more classes involved in the actual code. There are similar if/else clauses all ov开发者_如何学运维er the place.

I have been looking at the abstract factory pattern but I am having some problems.

Basically I am assuming that to avoid the if/else problems I need to pass the actual dervied types around. So BuildHistory should not use base types and maybe there should be multiple methods, one per derived type?

If you can't change the DTO classes perhaps you can try to subclass HistoryBuilder to deal with the different subclasses. Then you use the appropriate HistoryBuilderX to create a HistoryItem from a ResultItem. Then the question is how to get the appropriate HistoryBuilderX for the ResultItem supplied.

Still, if you can't change the BaseResultItem class to include a GetBuilder function you need to use some if..else if.. construct that inspects the classtypes of your ResultItems.

Or you create a Registry where every ResultItem class is registered with its corresponding HistoryBuilderX class. But that might be overkill.

The general 'design pattern' is simply to use object orientation with polymorphism instead of type checks. Thus: a BuildHistory method inside BaseResultItem, overridden by descendants.

Any code which checks the concrete type of an object smells (in a refactoring sense). Supporting different behaviours for different types is what OO is about.

Use polymorphism to remove the type checks.

if (result is ResultItem1)
{
    ((HistoryItem1)history).Property2 = ((ResultItem1)result).Property2;
}

Becomes then something like

result.addToHistory( history );

If for some reason, you don't want to scatter the logic in the item classes, have a look at the visitor pattern. In this case, you have something like:

public class Visitor {
     History history;
     public visit ( ResultItem1 item )  { ... }
     public visit ( ResultItem2 item )  { ... }
     ...
}

public class ResultItem1 {
     public accept( Visitor v ) { v.visit( this ); }
}

The typecheck is removed by the double-dispatch in the visitor, which is slightly more elegant.

I didn't understood exactly how the various kind of history relates to the various kind of items. So this is just a sketch of possibles direction to follow.