3

High level: I think my design pattern is flawed. Despite implementing Polymorphism, I find myself relying on large switch statements based on derived Type within my WPF application. As I'm adding more and more classes derived from my abstract base class I can't help but feel like my code it bad.. spaghetti code if you will

Example: Let's say I have a abstract base class called: PDF.cs. I have to implement 35 different derived classes from PDF.cs. The user picks 1 one of these derived classes from a drop down list on page 1 of the app. As the app progresses from page to page the user adds unique properties and values to the PDF type they are making. The pages are shared between types, however different controls are rendered and different properties are assigned depending on the type selected on page 1. The user is also allowed to navigate backwards and forwards between pages, with the data maintaining state.

My implementation:

// Pdf was passed to this page, handling UI
public void UtilizeState(IPdfBase pdf) {
    switch (pdf.PdfType)
    {
        case PdfTypes.TypeA
            var aPdf = (a)pdf;
            this.CurrentPdf = aPdf;
            DoStuff_PdfA(aPdf);
            break;
        case PdfTypes.TypeB
            var bPdf = (b)pdf;
            this.CurrentPdf = bPdf;
            DoStuff_PdfB(bPdf);
            break;
    }

    // we could be moving backward, need to populate UI
    if(this.CurrentPdf.Reverse)
    {
        handleReverse(this.CurrentPdf);
    }
    else
    {
        handleNewPdf(this.CurrentPdf);
    {
}

// Moving to the next page
private void btnNext_Click(object sender, RoutedEventArgs e)
{       
        switch (CurrentPdf.PdfType)
        {
            case PdfType.TypeA:
                var aPdf = (PSI)CurrentPdf;
                Switcher.Switch(new UploadPdf(), aPdf);
                break;

            case PdfType.TypeB:
                var bPdf = (COD)CurrentPdf;
                Switcher.Switch(new UploadPdf2(), bPdf); // Some pdf's follow a different workflow..
                break;
        }
}

private void btnBack_Click(object sender, RoutedEventArgs e)
{
    // another switch...
}

Comments: As I progress through this project, stuff is getting messy. Especially with 3 huge switch statements per page, one to handle PDF types incoming, another to send PDF types backwards a page, and yet another to send PDF types forward a page. Is my design inherently wrong? How would you handle passing many derived objects, with different business rules and workflows, through out shared pages?

  • There has to be a decision made somewhere. – Robert Harvey Mar 29 '17 at 23:19
  • True, although I had flirted with a solution involving abstracted <T> methods (referencing some sort of dictionary containing a enum of PDF type -a prop of the base class) to handle page changing dynamically with no luck. – Andrew Deacy Mar 29 '17 at 23:51
4

A common suggestion is to put all the logic into each Pdf object. But this requires that you extend the interface of Pdf and amend the implementation of each Pdf object each time want to support a new operation. It also requires that each Pdf object has to know everything about how to operate on it, rather than just knowing how to store its own representation. The result is that you end up with giant Pdf objects that basically have to do everything.

You probably want to use the Visitor Pattern to accomplish this.

Standard Visitor Pattern

Add a new method to your Pdf base class:

class Pdf {
  ...
  public abstract void AcceptVisitor(IPdfVisitor visitor);
}

The IPdfVisitor interface is fairly simple:

interface IPdfVisitor {
  void Visit(FirstPdf pdf);
  void Visit(SecondPdf pdf);
  void Visit(ThirdPdf pdf);
}

Now, each Pdf class object you implement defines the AcceptVisitor function in the same way (but you need to write the implementation in each class).

class FirstPdf : Pdf {
  ...
  public override void AcceptVisitor(IPdfVisitor visitor) {
    visitor.Visit(this);
  }
}

So, why do you have to have the implementation in each class? Because the compiler knows the static type of each class. So when you call visitor.Visit(this) from an object of type FirstPdf, it will call the Visit function that accepts an argument of type FirstPdf. Now, you implement visitors for each "verb" you want to implement. Maybe you have a SavePdfVisitor, a PrintPdfVisitor, and a DisplayPdfVisitor, for example. They would look like this:

class SavePdfVisitor : IPdfVisitor {
  public void Visit(FirstPdf pdf) {
    // FirstPdf specific save logic
  }
  public void Visit(SecondPdf pdf) {
    // SecondPdf specific save logic
  }
  public void Visit(ThirdPdf pdf) {
    // ThirdPdf specific save logic
  }
}

Now, your generic save method for Pdfs looks like this:

public void Save(Pdf pdf) {
  IPdfVisitor saveVisitor = new SavePdfVisitor();
  pdf.AcceptVisitor(saveVisitor);
}

The pdf object calls Visit and passes itself to the save visitor, without needing to know what type of operation is being performed. And the correct method of SavePdfVisitor gets called. At a minimum, this lets you put all Save logic into a single place and use private methods for shared functionality.

One advantage of this is that if you define a new type of Pdf, when you implement its AcceptVisitor method, you will get a compiler error unless you add that type to the IPdfVisitor interface and then to each of the visitor objects. This guarantees that when you define a new type of Pdf, you don't forget to implement the logic for it for all the operations you perform on Pdf's.

Better Visitor Pattern (In My Opinion)

On the other hand, if you have a lot of shared logic -- for example, the save operation is the same for most Pdf types but is custom for just a few -- you can define your visitor objects using an abstract base class instead of an interface like this.

abstract class PdfVisitor {
  public abstract void Visit(Pdf pdf);
  public virtual void Visit(FirstPdf pdf) {
    Visit((Pdf)pdf);
  }
  public virtual void Visit(SecondPdf pdf) {
    Visit((Pdf)pdf);
  }
  public virtual void Visit(ThirdPdf pdf) {
    Visit((Pdf)pdf);
  }
}

class Pdf {
  ...
  public abstract void AcceptVisitor(PdfVisitor visitor);
}

class FirstPdf : Pdf {
  ...
  public override void AcceptVisitor(PdfVisitor visitor) {
    visitor.Visit(this);
  }
}
// Do the same for other Pdf classes

Notice that PdfVisitor does NOT define the implementation of Visit(Pdf pdf). It does, however, provide a default implementation of Visit for all the concrete implementation types. That default implementation just calls Visit(Pdf pdf). When you implement Visit(Pdf pdf) in each of your concrete visitor objects, that defines the default behavior for that visitor or action. For example, suppose that your DisplayPdfVisitor uses the same logic for every type of Pdf. You just implement it this way:

class DisplayPdfVisitor : PdfVisitor {
  public override void Visit(Pdf pdf) {
    // Display logic here
  }
}

On the other hand, if SavePdfVisitor does the same thing for all Pdf objects, except that SecondPdf needs a special implementation, do this:

class SavePdfVisitor : PdfVisitor {
  public override void Visit(Pdf pdf) {
    // default save implementation
  }
  public override void Visit(SecondPdf pdf) {
    // custom save implementation for SecondPdf type
  }
}

Now if you call pdf.AcceptVisitor(saveVisitor) on a Pdf object of type SecondPdf and it calls visitor.Visit(this), it will call the method that accepts an object of type SecondPdf. However, the same code path for any other type of Pdf will just call the default implementation.

The advantage of this approach is that you can define a new Pdf operation by creating a visitor that just implements a single method and it will apply to all object types. You can also define a new type of Pdf object and it will get default behavior on all your visitors without having to make any changes. If you create a new Pdf class, say FourthPdf, and you realize that you need custom logic for it in your DisplayPdfVisitor then you just modify the base class:

abstract class PdfVisitor {
  // previous stuff
  public virtual void Visit(FourthPdf pdf) { 
    Visit((Pdf)pdf);
  }
}

And then in DisplayPdfVisitor:

class DisplayPdfVisitor : PdfVisitor {
  // other implementation
  public override void Visit(FourthPdf pdf) {
    // custom FourthPdf display logic
  }
}

You didn't have to change SavePdfVisitor or PrintPdfVisitor or anything else. They gladly apply the default save and print behavior to FourthPdf objects, but DisplayPdfVisitor handles them differently.

  • Thanks Jeremy, this is brilliant. Took awhile for me to wrap my head around this but after implementing today my code is looking much much better. The better Visitor pattern was ideal for my use as PDF types share a lot of functionality. – Andrew Deacy Mar 30 '17 at 20:36
  • Glad it helps! It is a nice way to separate objects from actions you perform on those objects. The Wikipedia article is good. The Visitor Pattern is an example of double dispatch (the actual function that gets called depends on the dynamic type of two objects: the visitor and the pdf). Most object-oriented languages only support single dispatch. This is what virtual functions do in classes: the actual function called depends only on the dynamic type of the object you call it on. That's why this may be confusing at first: it isn't supported as simply as single dispatch and isn't as common. – Jeremy West Mar 30 '17 at 21:24
  • That's awesome, I'll be reading up more on double dispatch patterns tonight. Very clever design pattern. I'm now working on adding a current page context param into the into the AcceptVisitor method to handle some of the UI stuff, if that works as planned the ugly switch statements will be no more! – Andrew Deacy Mar 30 '17 at 22:04
3

Generally speaking, you can resolve this sort of thing by changing this:

DoStuff_PdfA(aPdf);

to this:

aPdf.DoStuff();

Each derived type can override DoStuff to get it to do something type-specific.

When you do it this way, DoStuff won't have access to the member variables in current scope, so you may have to pass it some sort of context:

aPdf.DoStuff(myContext);

or if you want something quick and dirty you can cheat and do something like this:

aPdf.DoStuff(this);

If you need to create a worker for aPdf (e.g. create a type-specific UploadPdf object), inject a factory

var uploader = aPdf.GetUploader(uploaderFactory);

...and let the subtype resolve it:

PdfUploader GetUploader(IUploaderFactory factory)
{
    return factory.Resolve<PdfTypeA>();
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.