Is there a design pattern to handle when code depends on the subtype of two objects

Question

I'll try to be as explicit as possible, in case there is a better solution for my problem than answering my question.

I'm working in C#.

I have a report template that can include any number of 'features' turned on. A feature might be a table of information, a pie/bar chart, a list, etc. I am generating the report as a text file, or a PDF (possibly other options in the future).

So far I have an IFeature interface, and some feature types implementing it: ChartFeature, ListFeature, etc. I read the list of features enabled from the database and pass each one to a method along with the data id and the method returns a populated IFeature of the proper type.

I also have an IReportWriter interface that TextReportWriter and PdfReportWriter implement. That interface has a method: AddFeature(IFeature).

The problem is that AddFeature in each writer ends up looking like:

public void AddFeature(IFeature)
{
    InsertSectionBreakIfNeeded();

    if(IFeature is TableFeature)
    {
        TableFeature tf = (TableFeature)feature;
        streamWriter.WriteLine(tf.Title);
        for(int row=0; row < tf.Data.First.Length; row++)
        {
            for(int column=0; i < tf.Data.Length; i++)
            {
                if(i != 0)
                {
                    streamWriter.Write("|");
                }
                streamWriter.Write(feature.Data[column][row]);
            }
        }
    }
    else if(IFeature is ListFeature)
    {
        ListFeature lf = (ListFeature)feature;
        streamWriter.Write(lf.Title + ": ");
        bool first = true;
        foreach(var v in lf.Data)
        {
            if(!first)
            {
                streamWriter.Write(", ");
            }
            else
            {
                first = false;
            }
            streamWriter.Write(v);
        }
    }
    ...
    else
    {
        throw new NotImplementedException();
    }
    sectionBreakNeeded = true;
}

In the PDF writer the above would be modified to generate PDF table cells, text boxes, and so forth.

This feels ugly. I like it somewhat better as AddFeature(ListFeature){...}, AddFeature(ChartFeature) because at least then it's compile time checked, but in practice it just moves the problem so now outside if the IReportWriter I'm calling if(feature is ...).

Moving the display code into the feature just reverses the problem because it would need to know whether it should be writing plain text or a PDF.

Any suggestions, or am I best just using what I have and ignoring my feelings?

Edit: Filled in some of the conditions to give people a better idea of what is happening. Don't worry too much about the exact code in those examples, I just wrote it off the top of my head.

Answer 1

The general case of your problem is called double-dispatch - you need to dispatch to a method based on the runtime type of two parameters, not just one (the "this" pointer).

One standard pattern to deal with this is called the Visitor pattern. It's description traces back to the original Design Patterns book, so there's lots of example and analysis of it out there.

The basic idea is that you have two general things - you have the Elements (which are the things that you're processing) and Visitors, which process over the Elements. You need to do dynamic dispatch over both of them - so the actual method called varies depending on both the concrete type of the element and of the visitor.

In C#, and kinda sorta following your example, you'd define an IFeatureVisitor interface like this:

public interface IFeatureVisitor {
    void Visit(ChartFeature feature);
    void Visit(ListFeature feature);
    // ... etc one per type of feature
}

Then, in your IFeature interface, add an "Accept" method.

public interface IFeature {
    public void Accept(IFeatureVisitor visitor);
}

Your feature implementations would implement the Accept method like so:

public class ChartFeature : IFeature {
    public void Accept(IFeatureVisitor visitor) {
        visitor.Visit(this);
    }
}

And then your report writers would implement the IVisitor interface and do whatever it's supposed to do in each type.

To use this, it's look something like this:

var writer = new HtmlReportWriter();
foreach(IFeature feature in document) {
    feature.Accept(writer);
}
writer.FinishUp();

The way this works is that the first virtual call to Accept resolves back to the concrete type of the feature. The call to the Visit method is NOT virtual - the call to visitor.Visit(this) calls the correct overload since at that point it knows the exact static type of the thing that's being visited. No casts and type safety is preserved.

This pattern is great when new visitor types get added. It's much more painful when the elements (features in your case) change - every time you add a new element, you need to update the IVisitor interface and all the implementations. So consider carefully.

As I mentioned, there's been almost 20 years since the book was published, so you can find lots of analysis and improvements on Visitor pattern out there. Helpfully this gives you enough of a start to continue your analysis.

Answer 2

I would structure this in a slightly different way:

I wod have a IReport object that composes all the features in the report. This object would have methods AddFeature(IFeature) and GenerateReport(IReportWriter)

I would then have IFeature implement WriteFeature(IReport, IReportWriter) and this way delegate how the Feature is actually processed to the Feature itself.

The way you've structured the code makes me think that there is no way to write a Feature in a format agnostic way that can be processed by any given writer, so let the object itself deal with the issue.