15

I'm building a WPF application using the MVVM pattern. Right now, my viewmodels calls the service layer to retrieve models (how is not relevant to the viewmodel) and convert them to viewmodels. I'm using constructor injection to pass the service required to the viewmodel.

It's easily testable and works well for viewmodels with few dependencies, but as soon as I try to create viewModels for complex models, I have a constructor with a LOT of services injected in it (one to retrieve each dependencies and a list of all available values to bind to an itemsSource for example). I'm wondering how to handle multiple services like that and still have a viewmodel that I can unit test easily.

I'm thinking of a few solutions:

  1. Creating a services singleton (IServices) containing all the available services as interfaces. Example: Services.Current.XXXService.Retrieve(), Services.Current.YYYService.Retrieve(). That way, I don't have a huge constructor with a ton of services parameters in them.

  2. Creating a facade for the services used by the viewModel and passing this object in the ctor of my viewmodel. But then, I'll have to create a facade for each of my complexe viewmodels, and it might be a bit much...

What do you think is the "right" way to implement this kind of architecture ?

alfa-alfa
  • 153
  • 1
  • 1
  • 4
  • I think the "right" way to do it is to create a separate layer that calls the services and does whatever casting is necessary to create the ViewModel. Your ViewModels should not be responsible for creating themselves. – Amy Blankenship Nov 04 '13 at 18:29
  • @AmyBlankenship: View Models shouldn't have to (or even necessarily be able to) create themselves, but inevitably will sometimes be responsible for creating *other view models*. An IoC container with automatic-factory support is a huge help here. – Aaronaught Nov 04 '13 at 18:52
  • "Will sometimes" and "should" are two different animals ;) – Amy Blankenship Nov 04 '13 at 19:34
  • @AmyBlankenship: Are you suggesting that view models shouldn't create other view models? That's a tough pill to swallow. I can understand saying that view models shouldn't use `new` to create other view models, but think of something as simple as an MDI application where clicking on a "new document" button or menu will add a new tab or open a new window. The shell/conductor *must* be able to create new instances of *something*, even if it's hidden behind one or a few layers of indirection. – Aaronaught Nov 04 '13 at 21:15
  • Well, certainly it must have the ability to request that a View be made somewhere. But to make it itself? Not in my world :). But then again, in the world I live in, we call VM "Presentation Model." – Amy Blankenship Nov 04 '13 at 23:15

3 Answers3

26

In fact, both of these solutions are bad.

Creating a services singleton (IServices) containing all the available services as interfaces. Example: Services.Current.XXXService.Retrieve(), Services.Current.YYYService.Retrieve(). That way, I don't have a huge constructor with a ton of services parameters in them.

This is essentially the Service Locator Pattern, which is an anti-pattern. If you do this, you will no longer be able to understand what the view model actually depends on without looking at its private implementation, which will make it very difficult to test or refactor.

Creating a facade for the services used by the viewModel and passing this object in the ctor of my viewmodel. But then, I'll have to create a facade for each of my complexe viewmodels, and it might be a bit much...

This isn't so much an anti-pattern but it is a code smell. Essentially you're creating a parameter object, but the point of the PO refactoring pattern is to deal with parameter sets that are used frequently and in a lot of different places, whereas this parameter would only ever be used once. As you mention, it would create a lot of code bloat for no real benefit, and wouldn't play nice with a lot of IoC containers.

In fact, both of the above strategies are overlooking the overall issue, which is that coupling is too high between view models and services. Simply hiding these dependencies in a service locator or parameter object does not actually change how many other objects the view model depends on.

Think of how you would unit-test one of these view models. How big is your setup code going to be? How many things need to be initialized in order for it to work?

A lot of people starting out with MVVM try to create view models for an entire screen, which is fundamentally the wrong approach. MVVM is all about composition, and a screen with many functions should be composed of several different view models, each of which depends on only one or a few internal models/services. If they need to communicate with each other, you do so via pub/sub (message broker, event bus, etc.)

What you actually need to do is refactor your view models so that they have fewer dependencies. Then, if you need to have an aggregate "screen", you create another view model to aggregate the smaller view models. This aggregate view model doesn't have to do very much by itself, so it in turn is also fairly easy to understand and test.

If you've done this properly, it should be obvious just from looking at the code, because you'll have short, succinct, specific, and testable view models.

Aaronaught
  • 44,005
  • 10
  • 92
  • 126
  • Yeah, that's what I'll probably end up doing ! Thanks a lot, sir. – alfa-alfa Nov 04 '13 at 19:20
  • Well, I already assumed he already tried this, but didn't succeed. @alfa-alfa – Euphoric Nov 04 '13 at 19:35
  • @Euphoric: How do you "not succeed" in this? As Yoda would say: Do or do not, there is no try. – Aaronaught Nov 04 '13 at 20:48
  • 1
    @Aaronaught For example he really needs all the data in single viewmodel. Maybe he has grid and different columns come from different services. You cannot do that with composition. – Euphoric Nov 04 '13 at 20:57
  • 1
    @Euphoric: Actually, you *can* solve that with composition, but that can be done beneath the view model level. It's simply a matter of creating the right abstractions. In that case, you only need one service to handle the initial query to get a list of IDs, and a sequence/list/array of "enrichers" which annotate with their own information. Make the grid itself its own view model, and you've solved the problem with effectively two dependencies, and it's extremely easy to test. – Aaronaught Nov 04 '13 at 21:13
  • How is `try to create view models for an entire screen, which is fundamentally the wrong approach` different from `to have an aggregate "screen", you create another view model to aggregate the smaller view models`? Are you saying that the **screen** VM directly has all the properties that the smaller view models need? Or that it actually has all the smaller VMs are properties? – Jeff Sep 07 '17 at 02:14
2

I could write a book about this...in fact I am ;)

First off, there is no universally "right" way to do things. You have to take other factors into consideration.

It's possible that your services are too fine grained. Wrapping the Services with Facades that provide the interface that a specific Viewmodel or even a cluster of related ViewModels would use might be a better solution.

Simpler still would be to wrap the services into a single Facade that all viewmodels use. Of course that can potentially be a very big interface with a lot of unnecessary functions for the average scenario. But I would say it's no different than a message router that handles every message in your system.

In fact, what I've seen a lot of architectures eventually evolve into is a message bus built around something like the Event Aggregator pattern. Testing is easy there because your test class just registers a listener with the EA and fires the appropriate event in response. But that's an advance scenario that takes time to grow to. I say start with the unifying facade and go from there.

Michael Brown
  • 21,684
  • 3
  • 46
  • 83
  • A massive service façade is *very* different from a message broker. It's almost at the opposite end of the dependency spectrum. A hallmark of this architecture is that the sender knows nothing about the receiver, and there may be many receivers (pub/sub or multicast). Perhaps you're confusing it with RPC-style "remoting" which is just exposing a traditional service over a remote protocol, and the sender is still coupled to the receive, both physically (endpoint address) and logically (return value). – Aaronaught Nov 04 '13 at 20:48
  • The similarity is that the facade acts like a router, the caller doesn't know which service/services handles the call just like a client sending a message doesn't know who handles the message. – Michael Brown Nov 04 '13 at 20:57
  • Yes, but the façade is then a [God Object](http://en.wikipedia.org/wiki/God_object). It has all of the dependencies that the View Models have, probably more because it's going to be shared by several. In effect you've eliminated the benefits of loose coupling that you worked so hard for in the first place, because now, whenever something touches the mega-façade, you have no clue what functionality it really depends on. Picture writing a unit test for a class that uses the façade. You create a mock for the façade. Now, which methods do you mock? What does your setup code look like? – Aaronaught Nov 04 '13 at 21:04
  • This is *very* different from a message broker because *the broker also knows nothing about the implementation of the message handlers*. It uses IoC under the hood. The façade knows everything about the recipients because it has to forward calls to them. The bus has zero coupling; the façade has obscenely high efferent coupling. Almost everything that you change, anywhere, will affect the façade as well. – Aaronaught Nov 04 '13 at 21:07
  • I think part of the confusion here - and I see this quite a lot - is just what a *dependency* means. If you have a class that depends on one other class, but calls 4 methods of that class, it has 4 dependencies, not 1. Putting it all behind a façade doesn't change the number of dependencies, it just makes them harder to understand. – Aaronaught Nov 04 '13 at 21:08
  • I didn't say it was the perfect solution, the facade should only dispatch the message to the appropriate service. I think the best architectures evolve from friction. Once people have friction with a single facade, they will want to break it down into more focused services. But these would be scenario based services rather than the fine grained services that the OP currently has. The facade should be protected from changes to the details of its services because those details are hidden behind the service. It's an extreme approach but fundamentally better than working from the other way. – Michael Brown Nov 04 '13 at 21:25
  • I understand your argument; what I'm arguing is that the façade doesn't make the design better, it makes it worse. You have exactly the same number of dependencies, but now they're harder to keep track of. It would be far better to just leave the design the way it is, and have the pain reminding you that the view models aren't following the SRP, vs. chucking it all into a façade and instantly losing track of what really depends on what else. – Aaronaught Nov 04 '13 at 21:35
  • Also, I would take issue with the statement that "the best architectures evolve from friction". Architectures don't evolve, they are designed. *Code* evolves, and it *can* evolve because it can be *refactored*. However, *re-architecting* is a significantly more painful proposition than *refactoring*, and if you don't pick the right architecture to start with, you're going to be fighting an uphill battle for a very long time. It's pretty easy to move a method into a different type, but you can't switch from Oracle to Riak in the middle of a project. – Aaronaught Nov 04 '13 at 21:36
0

Why not combine both?

Create a facade and put all services your viewmodels use. Then you can have single facade for all yourviewmodels without the bad S word.

Or you can use property injection instead of constructor injection. But then, you need to ensure that those are getting injected properly.

Euphoric
  • 36,735
  • 6
  • 78
  • 110