Putting logic in ViewModel get'ers

Question

What do you think about putting Get-logic in the getters of a ViewModel? Something like:

public class DummyViewModel
{
    public int Id { get; set; }

    private DummyObject myObject;

    public DummyObject MyObject
    {
        get
        {
            if (MyObject == null)
            {
                DummyRepository repo = new DummyRepository();
                myObject = repo.Get(Id);
            }
            return myObject;
        }
    }

}

Is this bad practice, or totally fine? I find my controllers getting really bloated by doing all the get-logic there, but I'm really torn as to where I should put it...

My reason for doing it this way, is that I can pass the ViewModel to different types of view, and only the neccessary DB-lookup will be performed based on what property is requested.

Answer 1

There is nothing wrong with putting logic in getters in VMs - the role of a VM is to present data to a view, and it should be as ready to "view" as possible (the View should not be having to do too much (if any) work to shape the data before showing it).

As an example, i use properties called GetAvailableClients in my VM, and that will be one of the properties that the View binds to. The job of that particular getter is to filter data - IOW present a reduced set of data selected from a complete list (which is also held in the VM), that data will usually be filtered using LINQ, which means i have placed some custom logic in the getter.

What i am not a fan of though is the rest of your approach, where if the property hasn't been filled it goes off to the repository and gets the data itself. To me that is a no-no, the property is totally violating the principle of single responsibility by making the property responsible for too much. Not to mention that is not a good practice to follow once you start binding that property to the UI - suddenly your app will start hanging when the user performs an action because your property getter has been triggered and it has decided to make a call to the database or a webservice, and to make it worse that call has been done on the UI thread.... it just gets ugly :)

Answer 2

Your question is generic, but i'll answer as best as I can.

I don't like these things:

1) Testability. Your property is creating the repository, how are you going to mock up your repository and test this?

2) Lazy loading. Lazy loading is a potential performance hit and the viewmodel should not be doing it. What happens if you bind your viewmodel to a grid with hundreds of entries?

3) Exposing Id. Your Id property (which I am assuming is the primary key value of the entity) has a setter. Are you going to present this Id in the view? If not, get rid of it, if so, remove the setter. The setter implies that the view must perform some sort of biz funtionality to look up the correct value and this breaks SoC.

Answer 3

Personally, I do not include any logic in my view models - they are pretty much dumb DTOs. I certainly wouldn't make a VM responsible for it's own loading via a repository.