0

I have a BaseViewModel which is inherited by multiple ViewModel classes. In my BaseViewModel I have a couple of dependencies which get injected from ViewModel. Now if I need to add a new dependency in my BaseViewModel I need to change all the VM which inherit BaseViewModel. Please let me know how can it be handled in Simple Injector. Following is my code structure:

How can I make my base class injection independent so that I don't need to make changes in all my inherited class?

Code:

public class BaseViewModel
{
    protected readonly IAESEnDecrypt AESEnDecrypt;
    protected readonly IDataService DataService;
    protected readonly INavigationService NavigateToPage;
    public BaseViewModel(INavigationService nav, IDataService data, IAESEnDecrypt encrypt)
    {
        AESEnDecrypt= encrypt;
        NavigateToPage = nav;
        DataService = data;
    }
}


public class ViewModel
{
   public ViewModel(INavigationService nav, IDataService data, IAESEnDecrypt encrypt) : base (nav, data, encrypt)
   {

   }
}

My BaseViewModel Contains some of the following Interfaces whose implementation is injected through constructor:

- NavigationService
- DataService
- GeoLocationService
- SmartDispatcher
- MessageBus which implement Message Aggregator

It also Contains some common properties as static variables whose data is used throughout the application like UserDetails. And also contains CancellationToken, IsBusy to display progressbar.

BaseViewModel also contain HandleException method which handle all the incoming exceptions from all ViewModel. Also Contains some common Commands which are used in all the Views like Si gnoutCommand, NavigationBar Commands.

Actually it has started to contain all kinds of common methods used among various ViewModel.

Please suggest how can i refactor this code?

Balraj Singh
  • 3,381
  • 6
  • 47
  • 82
  • Please format your code properly. – Yuval Itzchakov Feb 03 '15 at 09:50
  • I totally agree with the answer of Steven. It should not be necessary to use a base class. To guide you in the right direction, can you explain what functionality is in your baseviewmodel? Which methods etc. – Ric .Net Feb 03 '15 at 12:14
  • Hi Ric. I have added more details about my BaseViewModel can you please suggest how can i refactor this and improve it further? – Balraj Singh Feb 03 '15 at 19:10
  • Don't forget to ping @Ric.Net by using the @ sign. – Steven Feb 03 '15 at 19:14
  • @Ric.Net let me know your inputs. – Balraj Singh Feb 03 '15 at 19:15
  • @BalrajSingh is it a typical LoB application? What I mean is, is the application mainly used for doing CRUD operation maybe together with some business logic, some nice features for showing the data? – Ric .Net Feb 03 '15 at 20:49
  • @Ric.Net: Its a B2C application as a product from the company. It has features like Map with multiple points marked, booking items through a multiple user control and more. It does CRUD operation but with it also displays data with animated objects. – Balraj Singh Feb 04 '15 at 09:39
  • @Ric.Net The application that i am creating is similar to this application: http://www.windowsphone.com/en-in/store/app/olacabs/e413000c-4519-40cc-a7f4-8b525959e8f0 – Balraj Singh Feb 04 '15 at 09:57

3 Answers3

9

Your last sentence:

Actually it has started to contain all kinds of common methods used among various ViewModel

Precisely describes your problem! As Steven already described, that you're building almost the complete application through a single base class. Thereby infringing the Open-Closed principle which you are heavinly experiencing now.

The trick is design your application around very small SOLID ViewModels of which you compose the application at runtime. By splitting the ViewModels and using a UserControl as your views you can compose big complicated views for the user, while you still get all the benefits from using a SOLID design. So let’s take a look at some of your different interfaces that you implement and some of the functions you ‘handle’ in the base class:

NavigationService

This sounds like a service which controls the flow in your application. This sounds to me like your mainview(model). You could create a single MainViewModel which as a single property, let’s say CurrentView.Assuming you’re using WPF you typically would bind this property to a ContentControl. The content of this control can be everything from a single TextBlock to a complete UserControl. The UserControls can still be very complicated as they could be composed of multiple child usercontrol and so on. Using a MVVM framework (like e.g. Caliburn Micro or MVVM Light) for this is optionally but will come in handy.

It could also be an application global service with some of kind of callback or delegate function to perform navigation to a certain View(Model). It is in any case an infrastructural part of your application that deserves it own class and shouldn't be put away in a base class.

DataService

A single dataservice was the way I worked for over 10 years. Every time I hit my head against the wall. There comes a point in time that you need something special which is not included in your dataservice and you will probably go through your complete code base to make the right adjustments. Speaking of the Open-Closed principle…

Than I learned about the Command/Handler and Query/Handler pattern. You can read about this here and here. Using this pattern everywhere you need data you just inject the correct IQueryHandler<,> and use it right there. Not every view(model) needs data and certainly not the same data. So why use a global DataService? This is will also improve your Lifetime management of your DBContext object.

HandleException

Why is your base class responsible for handling the exceptions of your viewmodel? What does the base class know about this exceptions? What does the base class do? Log the exception, show a message to the user (what kind of message?) and silently continue? Letting the application break down 3 minutes later and leaving a user ignorant of what happened? I.M.O. exception should not be catched if you didn’t expect them to be thrown in the first place. Than log the exception at an application level (e.g. in your Main), show an ‘Excuse me’ message to the user and close the application. If you expect an exception, handle it right there and then and handle according.

UserDetails

Ask yourself the question how many of your 40 ViewModels actually need this information? If all 40 are in need of this information, there is something else wrong with your design. If not, only inject this details (or even better an IUserContext) in the ViewModels that actually use them.

If you use it for some kind of authentication consider using a decorator wrapping the task they need permission for performing it.

IsBusyIndicator

Again: do you need this in every ViewModel? I think not. I think furthermore, showing the user a busy indicator is a responsibility of the View, not the ViewModel and the as the length of the task determines if you need to show this, make it a responsibility of the task (assuming you’re looking at your tasks also in a SOLID manner by using e.g. the already mentioned Command/Handler pattern).

With WPF you could define a Dependency Property that you can bind to the view, thereby showing some kind of busy indicator. Now just inject a ShowBusyIndicatorService in the task that needs to show it. Or wrap all your (lengthy) tasks in a ShowBusyIndicatorDecorator.

Design

Now let’s look at some simple interfaces you could define to build up your View(Model)s. Let’s say we decide to make every ViewModel responsible for a single task and we define the following (typical LoB) tasks:

  • Show (any kind of) data
  • Select or choose data
  • Edit data

A single task can be stripped down to ‘Show data of single datatype (entity)’. Now we can define the following interfaces:

  • IView<TEntity>
  • ISelect<TEntity>
  • IEdit<TEntity>

For each interface type you would create a Processor/Service or DialogHandler depending on your semantic preferences which would do the typical MVVM stuff like finding a corresponding view and binding this to viewmodel and show this in some way (a modal window, inject it as usercontrol in some contentcontrol etc.).

By injecting this single Processor/Service or DialogHandler in the your ‘Parent’ ViewModel where you need to navigate or show a different view you can show any type of entity by a single line of code and transfer the responsibility to the next ViewModel.

I’m using these 3 interfaces now in a project and I really can do everything I could do in the past, but now in SOLID fashion. My EditProcessor, interface and viewmodel look like this, stripped down from all not so interesting stuff. I’m using Caliburn Micro for the ViewModel-View Binding.

public class EditEntityProcessor : IEditEntityProcessor
{
    private readonly Container container;
    private readonly IWindowManager windowManager;

    public EditEntityProcessor(Container container, IWindowManager windowManager)
    {
        this.container = container;
        this.windowManager = windowManager;
    }

    public void EditEntity<TEntity>(TEntity entity) where TEntity : class
    {
        // Compose type
        var editEntityViewModelType = 
        typeof(IEntityEditorViewModel<>).MakeGenericType(entity.GetType());

        // Ask S.I. for the corresponding ViewModel, 
        // which is responsible for editing this type of entity
        var editEntityViewModel = (IEntityEditorViewModel<TEntity>)
                 this.container.GetInstance(editEntityViewModelType);

        // give the viewmodel the entity to be edited
        editEntityViewModel.EditThisEntity(entity);

        // Let caliburn find the view and show it to the user
        this.windowManager.ShowDialog(editEntityViewModel);
    }
}

public interface IEntityEditorViewModel<TEntity> where TEntity : class
{
    void EditThisEntity(TEntity entity);
}

public class EditUserViewModel : IEntityEditorViewModel<User>
{
    public EditUserViewModel(
        ICommandHandler<SaveUserCommand> saveUserCommandHandler,
        IQueryHandler<GetUserByIdQuery, User> loadUserQueryHandler)

    {
        this.saveUserCommandHandler = saveUserCommandHandler;
        this.loadUserQueryHandler = loadUserQueryHandler;
    }

    public void EditThisEntity(User entity)
    {
        // load a fresh copy from the database
        this.User = this.loadUserQueryHandler.Handle(new GetUserByIdQuery(entity.Id));
    }

    // Bind a button to this method
    public void EndEdit()
    {
        // Save the edited user to the database
        this.saveUserCommandHandler.Handle(new SaveUserCommand(this.User));
    }

    //Bind different controls (TextBoxes or something) to the properties of the user
    public User User { get; set; }
}

From you IView<User> you can now edit the current selected User with this line of code:

// Assuming this property is present in IView<User>
public User CurrentSelectedUser { get; set; }

public void EditUser()
{    
    this.editService.EditEntity(this.CurrentSelectedUser);
}

Note that by using this design you can wrap your ViewModels in a decorator to do crosscutting concerns, like logging, authentication and so on.

So this was the long answer, the short one would be: loose the base class, it is biting you and it will bite you more and harder!

Ric .Net
  • 5,540
  • 1
  • 20
  • 39
  • I got your point actually I was trying to do right think in wrong way. Thanks for your guidance. I have started working on the same. But as a start can you send me a sample code of this kind of implementation which would help me not to fall in any pit again. Looking for your response... – Balraj Singh Feb 04 '15 at 10:06
  • 1
    @Balraj_Singh The code in the answer has all code that is necessary for this design. The actual point is that you play with this to make a tailor made design based your demands and application domain. Giving you a copy of my design would give you the idea of an inflexible design as every application and domain has its own needs. So giving you an example is impossible for me because I simple do not have that knowledge – Ric .Net Feb 04 '15 at 20:59
  • I do understand. So I will try to change my code design as per this understanding and will ask you again if I get stuck with a problem. Thanks :) – Balraj Singh Feb 05 '15 at 06:19
  • @Balraj_Singh sure! Feel free to post another question and ping me! Have fun designing! – Ric .Net Feb 05 '15 at 20:24
  • As mentioned above I am trying to updated my current traditional implementation of MVVM to the one that you mentioned. But I am not able to get some of the thinks like: 1. Currently View Datacontext is binded to ViewModel hence all the controls are controlled by a property in VM. How would I change this to your above mentioned pattern with Processor/Service or DialogHandler? 2. I am using Delegatecommands which are binded to command property of UI element. Execution of these command certain action happens like animation, usercontrol is displayed. How to do it in command pattern? – Balraj Singh Feb 08 '15 at 06:57
  • It is recommended to ask a new question, because comments easily get unnoticed! In short: You can still set the viewmodel as the datacontext of your view (preferly in xaml). For real UI stuff take look at XAML styles and triggers. You can do almost completely in XAML, maybe together with some dependency properties or converters – Ric .Net Feb 08 '15 at 21:20
  • I have also asked the question can you please look at this link: http://stackoverflow.com/questions/28398363/change-current-implementation-of-basic-mvvm-to-adhere-to-solid-pattern/28398607?noredirect=1#comment45133447_28398607 – Balraj Singh Feb 09 '15 at 03:36
4

Prevent having this base class in the first place. This base class is a big code smell and the result is your current pain. Such a base class will violate the Single Responsibility Principle (SRP) and will just act as a big helper class for all derived view models, or it even seems that you are putting cross-cutting concerns in there. The base class might even hide the fact that your view models violate the SRP. They probably do too much; have too many responsibilities.

Instead, try to do the following:

  • Move cross-cutting concerns out of the base class into decorators or find another way to apply cross-cutting concerns.
  • Group related dependencies together into a aggregate service and inject such aggregate service into your view model.

In a well designed application, there is hardly ever a need for having such base class that takes dependencies.

If you aren't able to change your design (but please do take a look it this; you will be in a much better place without that base class), you can revert to explicit property injection. Simple Injector does not do this out-of-the-box, but the documentation describes how to do this.

Basically, it comes down to writing a custom IPropertySelectionBehavior, moving the constructor dependencies of the BaseViewModel to public properties and marking them with a custom attribute.

But again, only use property injection as a last resort. Property injection will only hide the design problem; it will not solve it.

Steven
  • 166,672
  • 24
  • 332
  • 435
  • Thanks Steven for your guidance. Can you share some links for Well design MVVM Applications. Where I can study and restructure my application? – Balraj Singh Feb 03 '15 at 11:53
  • @BalrajSingh: I'm not experienced with MVVM, but you might want to answer Ric .NET's question in the comments. He actually is quite experienced with MVVM, WPF and Caliburn, and he might have some advice for you. – Steven Feb 03 '15 at 18:56
0

You can use the ServiceLocator (anti)pattern to make the injection independent, HOWEVER you should not do this as it violates the principles of SOLID. Mark Seemann - Service Locator violates SOLID

You should rather stick to adding the dependencies in the constructor as this is in line with SOLID OO design principles.

toadflakz
  • 7,764
  • 1
  • 27
  • 40
  • yes i got your point but isn't there any way by which i can make the injection independent in Baseview. Actually I have 40 VM in which this BaseVM is inherited and going futher it may increase that is why i am trying to find an alternative for this. – Balraj Singh Feb 03 '15 at 09:54
  • 2
    You could use Property Injection instead of Constructor Injection but read the Simple Injector documentation on Property Injection - https://simpleinjector.readthedocs.org/en/latest/advanced.html#property-injection - You may want to consider evaluating your VMs for Single Responsibility Principle and redesign if necessary. – toadflakz Feb 03 '15 at 10:05