0

I'm working on an MVVM project and trying to preserve separation of concerns. Our current architecture has an entity framework model and MVVM light view and viewmodel projects. I'm working ViewModel first so I'm passing those around with a datatemplate in the view project to get the proper view.

Having to type check every type to get the correct viewmodel just seems wrong. Is there a better way?

To set this up suppose I have a model with classes:

namespace FoodData
{
    public class Hamburger:EntityObject
    {
            ..... //Add properties
    }

    public class Taco:EntityObject
    {
           .... //Add properties
    }

}

Now Lets say in my ViewModel namespace I have two different ViewModels for Taco and Hamburger

namespace FoodViewModel
{
  public class HamburgerViewModel : ViewModelBase
  {
  }

  public class TacoViewModel : ViewModelBase
  {
  }

  public class FoodViewModel : ViewModelBase
  {

    private ViewModelBase _displayViewModel;

    public ViewModelBase DisplayViewModel
    {
       get {return _displayViewModel;}
       set 
        {
            _displayViewModel = value;
            RaisePropertyChanged(nameof(DisplayViewModel));
        }
     }

     private EntityObject _selectedFood;

     public EntityObject SelectedFood
     {
        get{return _selectedFood;}
        set
        {
          _selectedFood = value;
          DisplayViewModel = _selectedFood?.GetViewModel();
          RaisePoropertyChanged(nameof(SelectedFood));
        }
     }

  }
}

The current solution I have is using extension methods in the FoodViewModel namespace

namespace FoodViewModel
{
   public static FoodViewModelExtensions
   {
       public static ViewModelBase GetViewModel(this EntityObject obj)
       {
            if(obj is Taco)
            {
              return new TacoViewModel();
            }
            if(obj is Hamburger)
            {
              return new HamburgerViewModel();
            }

            return new EmptyViewModel("Object not recognized");
       }
    }

}
  • 1
    This seems like a pretty obvious case for using an interface? Something like IHasViewModel<out T> { T GetViewModel(); } – Lee Feb 19 '18 at 13:16
  • So would the IHasViewModel be in the model's namespace? Could you provide an example of this? – Felix Castor Feb 19 '18 at 13:45
  • Yes, then class Taco : IHasViewModel<TacoViewModel> { ... } etc. – Lee Feb 19 '18 at 13:57
  • I'm trying to avoid references to my ViewModel in my Model. That is a solution but if I do it that way my Model would be dependent on my ViewModel. – Felix Castor Feb 19 '18 at 14:03
  • If you want to avoid that you could create a visitor interface IModelVisitor<TResult> { } which your models implement to replace the dynamic type switching. Then create a ViewModelVisitor : IModelVisitor<ViewModelBase> to create the view models. – Lee Feb 19 '18 at 14:09
2

What you have here between your models and viewmodels is a mapping.

No amount of code sorcery1 is going to change the fact that you're going to have to map pairs of (model+viewmodel) in order to decide which viewmodel to use for your model. As this is a given, the only real discussion left is how we want to define this mapping.

There are myriad ways to do so; most of which can be argued pro and con. We've already seen a few in answers, which I'll include in the list. Others are my suggestions.


  • (suggested by Lee) Letting the model implement a generically typed interface for the viewmodel (class Hamburger : IHasViewModel<HamburgerViewModel>)

This is the first thing I thought of too. However, I do agree that it feels a bit dirty for your model to have to know its related viewmodel (especially if you have several viewmodels for a given model).


  • (suggested by Felix Castor) Setting a Dictionary<ModelType,ViewModelType defined by letting the viewmodel implement a generically typed interface for the model (class HamburgerViewModel : FoodViewModelBase<Hamburger>)

The interface implementation has the same pros and cons as the previous bullet point, but now you have an added responsibility of having to use reflection to compose a dictionary on the fly?

I'm not a fan of this added complexity. The previous bullet point seems objectively better (due to less management) with no additional drawbacks.


  • (as a counter to Felix Castor's idea): why not simply maintain the type dictionary yourself, and do away with the interfaces and reflection?

Something like this:

public static Dictionary<Type,Type> ModelToViewModelDict = new Dictionary<Type,Type>()
{
    { typeof(Hamburger) , typeof(HamburgerViewModel) }
    { typeof(Taco) , typeof(TacoViewModel) }
}

This way, neither your model nor viewmodel class definitions are dirtied, because the mapping is maintained externally to the classes.

Having to maintain the dictionary isn't really an issue (as discussed in the beginning, the mapping needs to be maintained somewhere, in some shape anyway).


  • Using a visitor.

Unless I'm misunderstanding the example, this would require a similar generically typed interface and, on top of that, also require you to design a visitor who needs to be updated for every newly added food.

I don't see the benefits of this approach compared to the others. You end up with the same "dirty" model/viewmodel link by implementing an interface from one one the other; while also having to maintain an extra definition.

However, I'll admit that I have little experience with the visitor pattern in general, so I may be unaware of additional pro's (or cons).

Edit
The Visitor answer has been slightly updated since I wrote this, and I understand it a bit better now.
I still don't like it. It requires an overloaded method per entity type (which is very similar to your original FoodViewModelExtensions class); yet also requires every entity type to implement an interface (granted, it doesn't contain a generic type used for mapping purposes, but you are going to be copy/pasting the same interface method implementation all over your entities).

This feel like double work. A mapping can be achieved with only doing one of the two tasks that the visitor requires.


  • Don't overgeneralize your method parameters.

If you need to know the specific type of your passed object, then don't declare it as a shared base type. Instead, rely on overloading:

public static ViewModelBase GetViewModel(this Taco obj)
{
    return new TacoViewModel(obj);
}

public static ViewModelBase GetViewModel(this Hamburger obj)
{
    return new HamburgerViewModel(obj);
}

This comes with the added benefit that you can choose whether a default option (when the type is not known) can be given, or instead throw a compile-time error. If you want no default option to exist, then keep the code as above. However, if you want to allow the passing of types that are not explicitly mapped, you can add a default:

public static ViewModelBase GetViewModel(this EntityObject obj)
{
    return new EmptyViewModel(obj);
}

Note for the default method:
This is a bit of a pitfall. If you declare your entity as EntityObject myTaco = new Taco(); and then call GetViewModel(myTaco), you're going to end up in the default GetViewModel(this EntityObject obj) method; which is not what you'd want. This is because the type of the variable is used to decide which overloaded method to use.

To avoid this, make sure you declare your entity by its proper type: Taco myTaco = new Taco(); and then you will end up in the correct GetViewModel(this Taco obj) method.

Maintenance-wise, it's not all that different from your if chain. Originally, you needed to add a new if for every type, whereas now you need to add a new method for every type.

I like this version better, but I do concede that it at least partially relates to coding style preferences.

Note
Similar to how I advised against overgeneralizing your Hamburger and Taco objects as EntityObjects; there may be a similar argument to be made about having every method return a ViewModelBase as opposed to the specific HamburgerViewModel and TacoViewModel.

Even if you handle the resulting viewmodel the same way, you can still have the method return an object of type HamburgerViewModel but choose to store the return object in a ViewModelBase variable (thus not changing anything). So I would suggest keeping the return type sufficiently specific, and not overgeneralized.


  • Use a factory pattern.

This would be very similar to your current FoodViewModelExtensions class; with only a minor syntax change.


1I really wanted to come up with a suggestion that wouldn't require any manual maintenance when new entities get added to the codebase.

However, the best I could come up with is:

  • Using standardized naming across your model/viewmodel names, e.g. a Foo entity must have a viewmodel that is named FooViewModel.

You'll have to rely on reflection to find an object's corresponding viewmodel.Not having to manually maintain this is nice; but this is essentially a stringly typed approach; which I'm generally opposed to.

I'm less offended by the stringly typed approach than for other use cases; but it still feels a bit wrong to me. However, if you're really wanting to automate your mapping, this would effectively prevent you from having to manually maintain it.


To conclude

There are many ways to approach this. Each method has its own benefits, drawbacks, and applicability to the developer's preferred coding style (or the company's coding standards).

The most important thing to consider here is that you want your mapping to have a minimal footprint.

  • At an absolute minimum, using naming conventions to auto-map models and viewmodels is viable; but comes with some drawbacks in that the structure of an application shouldn't be defined by the names that are used.
  • A middle-of-the-road approach would require you to define the mapping once. There are two possibilities here:
    • Spreading the mapping over many locations (e.g. by having each model class definition list a IHasViewModel<HamburgerViewModel>)
    • Keeping the mapping contained in a single location (e.g. by using the dictionary, overloaded extension methods, or the factory). I much prefer this approach as it keeps the mapping centralized (inspired by SRP).
  • What you should definitely avoid is having the mapping be defined more than once. The posted example of the visitor comes pretty close to this by having you maintain both an interface mapping and a visitor class definition, both of which need to be updated when a new entity is added into the mix.

In the end, it's really just dealer's choice. Company coding standards (or those of your team lead) will likely take precedence. If not available, then it's up to you to decide which approach's drawbacks you're most willing to live with.

  • "To avoid this, make sure you declare your entity by its proper type: Taco myTaco = new Taco();" is what got me down this rabbit hole in the first place. Can't know before hand what concrete type of food needs to be displayed. – Felix Castor Feb 20 '18 at 14:17
  • Thanks for the answer! You had me at "No amount of code sorcery1 is going to change the fact that you're going to have to map pairs of (model+viewmodel) in order to decide which viewmodel to use for your model" – Felix Castor Feb 20 '18 at 14:19
  • @FelixCastor: That sounds more like you're dealing with overgeneralization of parameters in the calling code as well. When the object was initialized, you knew it was a Taco. But at this point, you seem to have lost that information and only know it's a EntityObject, which is the source of your issue. This is sort of why I advise against overgeneralization, you're now locked in an environment where it's impossible to know the original type, which is not a good approach. – Flater Feb 20 '18 at 14:20
  • @FelixCastor: As I mentioned in the answer, you should generally still be returning specific types from methods, even if you end up assigning the return value to an EntityObject parameter. But if you downcast the variable to a EntityObject, and then realize that you need to know its specific type later on, then you should not be downcasting the variable. Either remove the downcasting (better option), or you'll be stuck with your if(obj is Taco) chain (not a good option). – Flater Feb 20 '18 at 14:23
  • @FelixCastor: Can't know before hand what concrete type of food needs to be displayed. Maybe not before compilation, but the application will know at runtime. In order to facilitate correct handling at runtime, you're going to have to implement a correct handling for every possible type. Whether you solve that through generics, method overloading, or correct polymorphism is up to you; but given that you are trying to figure out the specific type of your base type object, that proves that your application's current polymorphism approach is not applied correctly. – Flater Feb 20 '18 at 14:27
1

One approach to separating the declaration of a type hierarchy from the traversal is to use a visitor e.g.

public interface IModelVisitor<TResult>
{
    TResult Visit(Hamburger h);
    TResult Visit(Taco t);
}

public interface IModel
{
    TResult Accept<TResult>(IModelVisitor<TResult> visitor);
}

public class Taco : IModel
{
    TResult Accept<TResult>(IModelVisitor<TResult> visitor) 
    {
        return visitor.Visit(this);
    }
}

then you can create a visitor for creating viewmodels:

public class ViewModelVisitor : IModelVisitor<ViewModelBase>
{
    public ViewModelBase Visit(Taco t) { return new TacoViewModel(); }
    public ViewModelBase Visit(Hamburger h) { return new HamburgerViewModel(); }
}

This approach requires to enumerate every model type in the visitor interface and add the Accept method to every model class. If you only have one visitor type this overhead may be unacceptable, and might prefer your existing approach, which you might be able to simplify with the pattern matching features in C#7.

  • Seems very similar to the extension methods. – Felix Castor Feb 19 '18 at 14:50
  • @FelixCastor - It's similar but statically ensures you handle every model type defined in the IModelVisitor<TResult> interface. It also separates the type switching from the handler behaviour. – Lee Feb 19 '18 at 17:14
0

The solution I came up with was in response to JimmyJames's comment. I made a Dictionary<Type, Type> that mapped the two and used reflection to build the map at runtime.

I had to make an interface to get the types using reflection:

public interface IFoodViewModel
{
}

Then I included an abstract class that implemented the interface:

public abstract class FoodViewModelBase<T> : ViewModelBase, IFoodViewModel
   where T : EntityObject
{
}

Then the concrete ViewModel classes become

public HamburgerViewModel : FoodViewModelBase<Hamburger>
{
    // Add properties to bind to
}

public TacoViewModel : FoodViewModelBase<Taco>
{
    // Add properties to bind to
}

In my above example the FoodViewModel needs two more methods and a field added.

First add a dictionary to map them:

Dictionary<Type, Type> _viewModelMapping;

Second a method to build the map called from the constructor in FoodViewModel.

       private void BuildEditViewModelMap()
        {
            _viewModelMapping = new Dictionary<Type, Type>();
            var types = AppDomain.CurrentDomain.GetAssemblies().SelectMany(s => s.GetTypes()).Where(p => typeof(IfoodViewModel).IsAssignableFrom(p) && !p.IsAbstract && !p.IsInterface);
            foreach (var x in types)
            {
                if (x.BaseType.GenericTypeArguments.Count() > 0)
                {
                    _viewModelMapping.Add(x.BaseType.GenericTypeArguments[0], x);
                }
            }
        }

And lastly a method to use the map to build ViewModels

private ViewModelBase GetViewModel(IDeviceModel device)
{
    // Note to add default arguments to create an instance add them to the object[] argument.
   var t = Activator.CreateInstance(_viewModelMapping[device.GetType()], new object[] {}) as ViewModelBase;
   return t;
}

So in conclusion, what does this buy me?

Now if I add a new food type say Salad to my model and need a new ViewModel to go with it, I just have to implement the abstract class FoodViewModelBase and FoodViewModel will handle the rest. I'm happy with this solution. Thanks for the help.

-2

I might be wrong, but why not try a strcture of type dynamic and get rid of these hardcoded case switch variants..

  • 1
    How would this solve getting the correct viewmodel based on the model object? – Felix Castor Feb 19 '18 at 13:44
  • You might want to combine Visitor pattern fron above answer with dynamic typing @Felix – rostamn739 Feb 19 '18 at 15:01
  • I would really like to keep strict typing in this project. I prefer to provide abstractions that contain a family of strictly typed concrete classes versus dynamic typing. – Felix Castor Feb 19 '18 at 15:05

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.