Architecture practices and refactoring legacy code in WPF.

Current state of things.

Recently I was given a task to redesign a WPF solution that was almost finished functionally and preparing for the shipping. As it often happens, this application started as a prototype that quickly added functionality and evolved into a colossus on feet of clay. Unfortunately, creator(s) of the application never really spent time to sit and think thoroughly about proper design, rushed by the urge to add features, bells and jingles and meet external requirements.
As a result, several flaws became obvious during the development phase and especially painful before the delivery of the product. To name a few, I smelled the following anti-patterns:

Big ball of mud.
This is the one that catches attention immediately. Lack of trained developers that turned on the projects frequently, lead to this BBoM. Quote from Wikipedia:

"Big ball of mud" systems have usually been developed over a long period of time, with different individuals working on various pieces. Systems developed by people with no formal architecture or programming training often create such systems.
Another cause of "big ball of mud" software is when managers put pressure on developers and ask them to write the system's code one part at a time and come with incremental micro requirements instead of providing a clear description of the problem to be solved.

God object.
This is another side of the same coin. Lack of initial design brought to enormous views and classes. Main (which was supposed to be a shell in terms of modular UI) and Login views and corresponding views bloated and following refactoring became very tiresome.

Cargo cult programming. Developers tried to use design patterns without proper knowledge of them and using them inappropriately.

Hard coding. Magic numbers, hard-coded file system paths, usernames and other environment variable items.

Repeating yourself. This is an inevitable consequence of "copy'n'paste" programming. Then, after some piece of functionality is already implemented and "works", developer switches to other part of (leaking) code and never returns for refactoring.

Spaghetti code. This is also inevitable when programming without having better principles (DRY, MVVM, KISS etc.) in mind. Putting UI logic into code-behind of the view will always end up in spaghetti code.

Programming by permutation This is also caused by copy'n'paste style of programming. Once it works in this specific circumstances, it's hard for a developer to foresee why this code would break on production.

Silver bullet Using third-part controls, popular libraries and tools unconsciously is pointless and can be even harmful.

This particular system is defective in several places.

  • It was developed without using MVVM, all logic was in code-behind.
  • Tightly coupled components.
  • No dependency injection.
  • No really unit tests and/or integration tests were created. Naturally, unit tests were hard to write due to tightly coupled elements of the system. Components just could not live in isolation.
  • Parallel development was complicated.
  • Extensibility, maintainability were complicated. Late binding were not possible.

That being said before us there was a complex task, or several tasks to be precise:

  • Implement SOLID
  • Loosely coupled architecture.
  • DI and object composition.
  • Proper MVVM.
  • Enable testability. Create unit tests.
Dependency injection. SOLID. Composition and patterns. Choice of IoC framework.

In the very beginning I can say that the great part of information about DI and related architectural patterns I found in this book by Mark Seemann and opuses by M. Fowler.

Briefly about advantages of DI:

  • Late binding: Services can be swapped with other services. Valuable in standard software, but perhaps less so in enterprise applications where the runtime environment tends to be well-defined.
  • Extensibility: Code can be extended and reused in ways not explicitly planned for. Always valuable.
  • Parallel development: Code can be developed in parallel. Valuable in large, complex applications; not so much in small, simple applications.
  • Maintainability: Classes with clearly defined responsibilities are easier to maintain. Always valuable.
  • TESTABILITY: Classes can be unit tested. Only valuable if you unit test (which you really, really should).

This is fair to say that every system that uses components (say every contemporary system) should use some kind of DI.
This can be either manually implemented resolver or some thirt-party container. Here are some of them: Castle Windsor, StructureMap, Spring.NET, Autofac, Unity, MEF.
All containers that are available are free so the cost is not a problem. We had to choose between several DI frameworks and decided to go with Ninject. It's open source and supports all features of DI we would like to have.

Code should comply with SOLID.

  • S: Single Responsibility Principle (SRP). A class should concentrate on doing one thing The SRP says a class should focus on doing one thing, or have one responsibility. This doesn’t mean it should only have one method, but instead all the methods should relate to a single purpose (i.e. should be cohesive).
    If your class has too many methods or has too many dependencies that is a code smell, class should be simplified or decomposed.
  • O: Open closed Principle (OSP). Open for extension (in derived classes) but closed for modification (of course you can change your class to fix bugs).
  • L: Liskov substitution Principle (LSP). We must make sure that new derived classes are extending the base classes without changing their behavior.
  • I: Interface Segregation Principle (ISP).Clients should not be forced to implement interfaces they don’t use. Instead of one fat interface many small interfaces are preferred based on groups of methods, each one serving one sub module.
  • D: Dependency Inversion Principle (DIP). High-level modules should not depend on low-level modules. Both should depend on abstractions. Abstractions should not depend on details. Details should depend on abstractions. High level module and Low level module keep as loosely coupled as much as we can.
    In terms of DI it can be translated as dependency should not be constructed in the service, but provided by the client. This would provide loose coupling.
MVVM framework. PRISM?

Once we have DI container in place, it's time to find useful MVVM framework for WPF. There are many third party frameworks nowdays but we decided to go with PRISM
Prism provides the main components of the modular desktop application and lots of sugar for XAML developers.
PRISM introduces concept of a Bootstrapper which is an initialization root of application. This is where the dependency should be registered (or container to be configured).

Also, Prism provides the following:

  • Modular application support (thru ModuleCatalog class and IModule interface)
  • It's own implementating of MVVM. Such as

public abstract class BindableBase : INotifyPropertyChanged

  • Synchronous and async version of ICommand implementation:
  • InvokeCommandAction
  • InteractionRequest
  • Region Manager
  • Inter-module communication e.g. EventAggregator.
Service Locator - pattern or anti-pattern?

Prism offers aforementioned Service Locator, which is kind of AbstractFactory and I've always been happy using Service Locator until recently, I read a post by by Mark Seemann where he describes the drawbacks of ServiceLocator pattern.
Basically, it violates SOLID and encapsulation. Instead constructor injection or other kind of injection should be used.
Service Locator violates the Interface Segregation Principle because client will be dependent on numbers of methods from the Locator that it does not really use.

However, it worth to be mentioned Service Locator is still much better than using Control Freak pattern where you just create new dependencies with a new statement.
public IProcess Process(Order order){...} is good because it uses a constructor injection

public IProcess Process(){ var locator = new Locator(); var order = locator.Resolve<IOrder>(); ...} is worse even if Locator uses dependency injection.

public IProcess Process(){ var order = new Order(); ...} is the worst, it creates a typical tightly coupled code. For example you will not be able to mock Order to test this class.

Components interaction. In-process messaging. Event Aggregator?

In some systems that are bigger than a Hello Word often there is a need for some message exchange mechanism. It holds for distributed systems where different components are located in different processes but it also true for in-process communication. It's hard to build a complex system which will not be tightly coupled and would be able to notify other parts of the system when something happens in the system e.g.

  • User login/logoff.
  • Network connected/disconnected. Notification should be displayed and some processes should be terminated gracefully.
  • User settings are changed. Language, culture, UI skin etc. All windows should be updated accordingly.
  • Scheduled event happens.
  • ...

Two approaches look reasonable in this scenario. Firstly, we can use Shared Services.
In this case we can divide the following services based on our examples:

  • UserService
  • NetworkService
  • SettingsService
  • CalendarService

Having each needed services injected into component that needs to be notified, we can remove hard references and
build event-based notifications. These services are usually instantiated as static to ensure they are shared amidst clients.

TDD. Design for testing. Isolation framework selection. MOQ.

Now, when we are positive about using DI and loosely-coupled architecture, there is time to choose a right mocking library.
There is choice between three options:

  • Rhino Mocks
  • Moq
  • NSubstitute

We found that Moq has a much more concise syntax but is missing some features that Rhyno Mock has like you may program your mock to return different results after consecutive runs. We were not actually interested in this functionality and decided to go with Moq.

Refactoring legacy code.