views:

219

answers:

1

I wanted to avoid the switch statement. I have over 30 document types. There is also a possibility I will need to add more document types moving forward. I would rather pass IDocument and have the type specified in the implementation of IDocument. Something else I forgot to mention was ProgressNoteViewModel, LabViewModel ... all inherit from WorkspaceViewModel and all of the concrete implementation constructors take a type IPatient as parameter. I am also using Castle as my IoC container

I would want to refactor the code to something like

viewModel = new TreeViewModel(repository.GetPatientDocumentListing(IDocumentType);
this.DocTreeViewModel = viewModel;
//How would I then be able to instantiate the right ViewModel
//based on IDocumentType and also pass a object into the
//constructor that is not know at compile time

I have the following code:

switch (docType)
{
    case "ProgressNotes":
        viewModel = new TreeViewModel(repository.GetPatientProgressNotes());
        this.DocTreeViewModel = viewModel;
        ProgressNoteViewModel workspace = ProgressNoteViewModel.NewProgressNoteViewModel(_patient);
        break;
    case "Labs":
        viewModel = new TreeViewModel(repository.GetPatientLabs());
        this.DocTreeViewModel = viewModel;
        LabViewModel workspace = LabViewModel.NewLabViewModel(_patient);
        break;
}
this.Workspaces.Add(workspace);
this.SetActiveWorkspace(workspace);
+1  A: 

Entirely untested:

public class ViewModelBuilderFactory
{
    public IViewModelBuilder GetViewModelBuilder (string docType, IRepository repository)
    {
        switch (docType)
        {
            case "ProgressNotes":
                return new ProgressNotesViewModelBuilder(repository);
            case "Labs":
                return new LabsViewModelBuilder(repository);
            default:
                throw new ArgumentException(
                    string.Format("docType \"{0}\" Invalid", docType);
        }
    }
}

public interface IViewModelBuilder
{
    TreeViewModel GetDocTreeViewModel();
    WorkSpace GetWorkSpace(Patient patient);
}

public class LabsViewModelBuilder : IViewModelBuilder
{
    private IRepository _repository;
    public LabsViewModelBuilder(IRepository repository)
    {
        _repository = repository;
    }

    public TreeViewModel GetDocTreeViewModel()
    {
        return new TreeViewModel(_repository.GetPatientLabs());
    }

    public Workspace GetWorkspace(Patient patient)
    {
        return LabViewModel.NewLabViewModel(patient);
    }
}

public class ProgressNotesViewModelBuilder : IViewModelBuilder
{
    private IRepository _repository;
    public ProgressNotesViewModelBuilder(IRepository repository)
    {
        _repository = repository;
    }

    public TreeViewModel GetDocTreeViewModel()
    {
        return new TreeViewModel(_repository.GetPatientProgressNotes());
    }

    public Workspace GetWorkspace(Patient patient)
    {
        return ProgressNoteViewModel.NewProgressNoteViewModel(patient);
    }
}

Now your calling code is:

ViewModelBuilderFactory factory = new ViewModelBuilderFactory();
IViewModelBuilder modelBuilder = factory.GetViewModelBuilder(docType, repository);
this.DocTreeViewModel = modelBuilder.GetDocTreeViewModel();
Workspace workspace = modelBuilder.GetWorkspace(patient);
this.Workspaces.Add(workspace);
this.SetActiveWorkspace(workspace);

[4 edits since first post; keep seeing mistakes]

[Further Edit noting that you're using Castle IOC]

In your Castle xml configuration, you could add (and I'm working on only a vague knowledge of Castle here)

<component id="ProgressNotesViewModelBuilder"
           type="MyNamespace.ProgressNotesViewModelBuilder, MyAssembly">
    <parameters>
        <!-- reference to repository here -->
    </parameters>
</component>
<component id="LabsViewModelBuilder"
           type="MyNamespace.LabsViewModelBuilder, MyAssembly">
    <parameters>
        <!-- reference to repository here -->
    </parameters>
</component>

Then you don't need the ViewModelBuilderFactory, you can just replace

IViewModelBuilder modelBuilder = factory.GetViewModelBuilder(docType, repository);

with

IViewModelBuilder modelBuilder = (IViewModelBuilder)
    container.Resolve(docType + "ViewModelBuilder");

Now you don't need your switch statement at all.

However, it's worth noting that switches aren't evil, they just smell bad and like all bad smells should be isolated from everything that smells good; that's what the Factory pattern is intended to achieve.

pdr
Yarg beaten by only a few minutes. You could also have each of the ModelBuilder classes implement the `IViewModelBuilder` interface. If you really want an IoC container involved you could use the docType to differentiate named configurations but might be a bit too much business logic configured into the Container.
smaclell
Meant to have them implement the interface, it makes no sense otherwise. Edited.
pdr
The change to fit in with the Inversion of Control Principle is to depend on the `IViewModelBuilder` instead of the concrete implementations `ProgressNotesViewModelBuilder` or `LabsViewModelBuilder`.
smaclell
Yeah, the IoC container is easy to put in if you know which container you're using. My thought was to assume there wasn't one and let the questioner decide how to make that change. It wasn't necessary to the question
pdr
Sorry I am new here. 2 2 I wanted to avoid the switch statement. I have over 30 document types. There is also a possibility I will need to add more document types moving forward. I would rather pass IDocument and have the type specified in the implementation of IDocument. Something else I forgot to mention was ProgressNoteViewModel, LabViewModel ... all inherit from WorkspaceViewModel and all of the concrete implementation constructors take a type IPatient as parameter. I am also using Castle as my IoC container
William
You will still need the switch, with IOC you are only moving the switch to select the strategy when the instance is is created rather than testing some "type" field to select your strategy in your runtime instance.
mP
@William - edited again for you
pdr