Unit Testing on Top of Entity Framework DbContext

27640565_sWhen writing unit tests one of the challenges is to isolate your tests from everything. To isolate them from the code that is not in their target and also from the other tests. As Roy Osherove puts it in his book “The Art of Unit Testing,  a unit test should always run in its little world, isolated from even the knowledge that other tests there may do similar or different things”. Test isolation makes a key difference between tests suites which are maintainable and bring high value and the ones which are a burden and bring little or no value.

In data intensive applications one of the most common difficulties when writing unit tests is to isolate them from the database. We would like that the tests which verify the business logic, not to hit the database, so we can easily configure test data for different test cases and we can easily assert the result.

One of the best approaches to achieve this, is to implement the Repository Pattern with a good abstraction that gives all the necessary functions to do data access. In my previous posts I present some examples. Having this, in our tests we can use stubs or mocks as implementations of the IRepository interface, and we can test the caller code in isolation. The code snippet below shows such a test which verifies that the where clause filters out data. Similarly, tests which verify that the result is ordered by a certain criteria, or that some business calculations are done correctly when the data is read or saved can easily be done.

[Test]
public void GetOrdersForShipment_AlsoPendingOrders_PendingOrdersFilteredOut()
{
  Order orderInPending = new Order {Status = OrderStatus.Pending};
  Order orderToShip = new Order {Status = OrderStatus.Processed};
  IRepository repStub = GetRepStubWith(orderInPending, orderToShip);

  var target = new OrderingService(repStub);

  IQueryable<Order> orders = target.GetOrdersForShipment();

  var expected = new[] {orderInPending};
  AssertEx.AreEquivalent(orders, expected);
 }

private IRepository GetRepStubWith(params Order[] orders)
{
  Mock<IRepository> repStub = new Mock<IRepository>();
  repStub.Setup(r => r.GetEntities<Order>())
         .Returns(orders.AsQueryable());

  return repStub.Object;
}

Entity Framework supports testable code designs and the DbContext in itself is a repository implementation. So what would it mean to stub or mock the DbContext directly and write isolated tests in a similar way as we did in the example above? We might need to do this when we don’t wrap the DbContext into another repository implementation or we want to test the code that does the wrapping (as we did in the DataAccess implementation here).

To get this going the first thing we need, is to make sure that the code we want to test does not use directly our specific context class, but its base class which is DbContext. A factory which will be used by the target code instead of newing up a MyDatabaseContext instance, gets this done. In the test code we will have the factory return a stub or a mock for the context.

Let’s start with a simple test: verify that filtering data at read works. So this simple test would look like this:

// target (production) code
class UsersService
{
  private DbContext dbContext;

  public User GetUserById(int id)
  {
     return dbContext.Set<User>()
                   .FirstOrDefault(x => x.Id == id);
  }
...
}

// unit test code, test method
...
 var contextFactory = GetFactoryWithTestData();
 var target = new UsersService(contextFactory);

 User actual = target.GetUserById(2)

 User expected = new User {Id = 2};
 Assert.AreEqual(expected, actual);

The test data is not visible here. This is because setting it up requires quite some code, and I’ve put it into the GetFactoryWithTestData() function. First, this function needs to build a stub for DbSet<User>, which contains a few user DTOs among which one has the Id == 2. Second, it has to build and configure a DbContext stub which returns the DbSet stub. In a simplified version the code looks like below:

...
private static DbContext CreateContextWithTestData()
{
  List<User> users = // newup a list with some users DTOs
  DbSet<User> userSet = GetDbSetStub(users);

  Mock<DbContext> contextStub = new Mock<DbContext>();
  contextStub.Setup(x => x.Set<User>())
             .Returns(() => userSet);

  return contextStub.Object;
}

...
private static DbSet<T> GetDbSetStub<T>(List<T> values) where T : class
 {
    return new FakeSet<T>(values);
 }
...
class FakeSet<T> : DbSet<T>, IQueryable where T : class
{
  List<T> values;
  public FakeSet(IEnumerable<T> values)
  {
     this.values = values;
  }

  IQueryProvider IQueryable.Provider
  {
    get { return values.AsQueryable().Provider; }
  }

  Expression IQueryable.Expression
  {
    get { return values.AsQueryable().Expression; }
  }

  Type IQueryable.ElementType
  {
    get { return values.AsQueryable().ElementType; }
  }

  public IList<T> Values
  {
    get { return values; }
  }

  public override T Add(T entity)
  {
    values.Add(entity);
    return entity;
  }

  public override T Remove(T entity)
  {
    values.Remove(entity);
    return entity;
  }
}

This works well for testing simple queries. For more complex scenarios, setting up data for one-to-many or many-to-many relations gets quite complex. You could set it once with a model of Users and Roles and use it for more tests, but it is hard to do the same for testing other areas of the application and all the business logic.

Another thing to notice in the above snippet is that we have written the FakeStub class instead of using Moq. This is because we want to keep some state on it (the values) and use it in test cases that involve adding or removing entities from the context.

Until at this point, we were able to stub or mock the DbContext and DbSet  because all the methods our code used, were overridable. This allowed our us or Moq to replace their behavior in the tests. However, not all public members (or their dependencies) of DbContext are like this, therefore it gets more difficult isolate the tests for some scenarios.

For example, if we would like to test the code that executes when an entity is read from the database, we would need to be able to raise the ObjectMaterialized event on the stubbed context. This event is not on the DbContext, but on the ObjectContext. The ObjectCotext property is nor public nor overridable, which makes it almost impossible to replace it with a stubbed ObjectContext on which we could trigger the event. To overcome this we can create a DbContext wrapper that just pushes up this event. Like this:

public sealed class DbContextWrapper : IDbContextWrapper
{
  private readonly ObjectContext objectContext;

  public DbContextWrapper(DbContext context)
  {
    Context = context;
    objectContext = ((IObjectContextAdapter) context).ObjectContext;
    objectContext.ObjectMaterialized += ObjectMaterializedHandler;
}

  private void ObjectMaterializedHandler(object sender, ObjectMaterializedEventArgs e)
  {
    EntityLoadedEventHandler handler = EntityLoaded;
    if (handler != null)
        handler(this, new EntityLoadedEventHandlerArgs(e.Entity));
  }

  public DbContext Context { get; private set; }

  public event EntityLoadedEventHandler EntityLoaded;

  public void Dispose()
  {
    objectContext.ObjectMaterialized -= ObjectMaterializedHandler;
    Context.Dispose();
  }
}

Now we need to modify all our test code and production code to use the IDbContextWrapper instead of the DbContext. The factory will return a stub for it and the stub can be configured to raise the event.

This is quite inconvenient. Our tests have too much knowledge of implementation details of the production code. Even more, when trying to tests more code that accesses data, things will get more complex and this wrapper will grow creating a hard to manage mess. It is also an example of how the tests may damage the production code design. Maybe by more refactoring this wrapper will lead to the IRepository interface as the abstraction of the repository pattern which hides EF from the production code, but… it seems unlikely and a very long an painful route.

All these point to the conclusion that for testing in isolation the business logic code, abstracting the data access in with some clear interfaces and testing from there up, is a better approach. Implementing the repository pattern on top of EF, not only gives better separation of concerns and may enforce consistency, but is also helps in testing.

However, testing on the DbContext directly may be useful when it is wanted to write some tests on the repository implementation that wraps the EF, and we want these tests to be isolated from the database and from the caller code. Such implementation is available on iQuarc github account, in the DataAccess repository.

 


more discussions about writing good unit tets are part of my Unit Testing Training

 


Featured image credit: geargodz via 123RF Stock Photo
Advertisements
Posted in .NET, Technical, Unit Testing | Tagged , , , , | Leave a comment

Repository Implementations

Shaping Repository ImplementationIn my previous post I have presented a way to separate your data access from the business logic, when a relational database is used. I have shown another implementation of the well-known Repository pattern. Since Martin Fowler described it in his book Patterns of Enterprise Application Architecture it became one of the most implemented pattern in enterprise applications. As it happens with all the design patterns, design principles or any other widely known best practice, we find them in many different implementations, mixed in different ways, but they all have identical things, because they all implement the same solution to a common problem. They all have classes and interfaces named the same. Thing like: Repository, UnitOfWork, IEntity, BaseEntity, ConcurencyException and so on.

I like the quote of Christopher Alexander in GoF (originally in his book A Pattern Language): “Each pattern describes a problem which occurs over and over again in our environment, and then describes the core of the solution to that problem, in such a way that you can use this solution a million times over, without ever doing it the same way twice”. In most of the projects I’ve been involved in, I’ve seen the Repository pattern implemented differently. Tweaked to the context of that project, to the specifics of that team, according with their own style. All different implementations of the same patterns and practices.

Out of the curiosity of how many other blog posts are out there, which show similar implementation with the one I’ve presented, I have googled: “generic repository implementation”. As expected there are quite a few, and as expected they are very similar with mine and with each other. I have randomly picked a few (the first 5 returned by my google search at the time of writing this article) that I will comment in this post.

Blog Post: Implement Step-by-Step Generic Repository Pattern in C#

I like the idea of defining an IEntity interface which should be implemented by all the entities that your repository can persist. This opens the possibility of writing a more generic code into a default implementation of IRepository<T>, (a Repository<T> class), where you could push all the common code like a generic implementation of .FindById(int Id). This will only work if you can enforce the convention that all the tables in your database have a surrogate primary key of type integer.

I appreciate a lot the discussions in the comments of the post. I am closer to the point of view of ardalis regarding the use of such abstraction on top of EF. However, I don’t see IQueriable<T> as a leaky abstraction. In my view it abstracts a query, which may be executed on different query providers, EF being one of them. It also helps a lot when Unit Testing, because I can stub the IRepository to return array.AsQueriable().

Blog Post: Generic Repository and Unit of Work Pattern, Entity Framework, Unit Testing, Autofac IoC Container and ASP.NET MVC [Part 1]

The implementation in this blog post also has the IEntity, but with a slight variation. It is generic by the type of the primary key. With this, we can have different types for the primary key, but it increases the complexity of writing generic code in a common repository implementation.

I like the IAuditableEntity, which should be implemented by all entities on which we want to store information like CreatedBy, CreatedDate etc. With this, in a generic implementation of the repository before we save an entity we can check if it implements IAuditableEntity and if yes, then we can set the correct values in CreatedBy, CreatedData properties and then continue with the save.

The implementation of the UnitOfWork looks a bit strange. It has a Commit() function, which should be called when the client wants to persist a set of changes. What I don’t like is that the client service will need to use one instance of a Repository, another instance of an UnitOfWork and these instances should be built in such way that they wrap the same DbContext instance. The way objects are created (Dependency Injection, Service Locator or custom factories), needs to take care of this, and all the client services need to be aware of it and use it consistently. Things may get too complex when a client service will need to use repositories of two different entities. It will have to deal with four dependencies only for this.

This implementation goes even further and defines a generic service for maintaining business entities. It is called EntityService. A suggestive name I’ve also used in different projects, for a similar generic service with the same scope. These set of services should use the repository (data access), encapsulate the logic around maintaining a business entity (business logic) and give to the controller (UI) functions for the basic CRUD operations.

Blog Post: Understanding Repository and Unit of Work Pattern and Implementing Generic Repository in ASP.NET MVC using Entity Framework

This blog post addresses the UnitOfWork implementation in a better way than the previous two. Here, the UnitOfWork has set of Repository instances that it creates and makes them available through get properties. This solves in a better way the thing that I didn’t like in the previous post. The DbContext is created and owned by the UnitOfWork, and because it is the one that creates the repositories it may pass it to the Repository class.

The drawback of this is that for a client class is quite difficult to know which are the repositories that are available on a specific UnitOfWork instance. It does not know what get properties to expect from an UnitOfWork. This may lead to inconsistency. As a client I might end up reaching the entities I want through strange paths starting from different repositories. It may also lead developers to add repository properties to UnitOfWork classes as they need them in different context, making the UnitOfWork classes to get fat and difficult to maintain.

The generic UnitOfWork class presented in the article helps, but even that may become quite complex when there are many specific repository implementations that need to be created and cached in the dictionary. Mainly the UnitOfWork turns into a Service Locator implementation for repositories of different entity types.

Blog Post: Generically Implementing the Unit of Work & Repository Pattern with Entity Framework in MVC & Simplifying Entity Graphs

This implementation has the same way of implementing a UnitOfWork as the previous. The UnitOfWork has a generic repository through a get property.

The article shows the idea of abstracting the DbContext under an IDbContext interface. With this, the UnitOfWork and Repository no longer depend on a specific context, but on this abstraction. This can help in separating the data access implementation in another assembly than the one in which we have generated the code for a specific context. This interface may be useful when you want that Dependency Injection or Service Locator to get directly involved on creating context instances and you need an interface for configuring the container. Otherwise, the same separation may be achieved with an abstract factory.

Another interesting thing is the RepositoryQuery<T>. The Repository.Get() does not return an IEnumerable nor an IQueryable. It returns this RepositoryQuery<T> which is somehow in the middle. It limits the queries which may be defined, by giving Include, Filter or OrderBy functions and when you say .Get() it composes the expressions and executes the query on the underlying repository.

It is hard to see the cases where the complexity added by this extra indirection pays off. It brings this common place where we can make sure that the queries written by upper layers may be adapted to work the same on any query provider, but unless we know that we need to easily replace EF with something else… might not worth doing it. Another advantage, I can think of, is that it enforces consistency in the client queries, but this needs to be balanced with the limitations it brings.

Blog Post: Implementing the Repository and Unit of Work Patterns in an ASP.NET MVC Application

This article is part of a tutorial on www.asp.net about getting started with EF and MVC. It presents the step by step the process of evolving from a simple repository, which does nothing more than wrapping a DbContext, towards a generic repository and then towards a unit of work that has more repository instances. It follows the same idea. An UnitOfWork class which provides a generic repository through a get property. The UnitOfWork takes care of creating and caching multiple repositories instances, one for each entity type. This may turn it into a complex factory.

Here, the Get() function of the repository returns an IEnumerable. It receives through input parameters different parts of a query. There are parameters to give a filter expression, an order by expression or a string to specify the related entities. In my view this is an unfortunate choice. It does not give to the client a fluent API to specify the query. The client code has to tweak the parameters in different ways which increases the complexity on its side. There will also be cases when new Get() overloads are needed to write in them the LINQ queries directly against the DbContext. This may lead to inconsistency. I would rather return IQueriable, or if not then have a Get() without parameters which returns IEnumeralble and explicitly ask for specific repository implementations which define other Get() overloads for the specific queries.

Conclusions

I think the biggest take away from these blog posts is that there are many ways we can implement the repository and unit of work patterns. All implementations will achieve the same goal which is separating data access concern from the other concerns. The implementations differ because they are used in different context. All have pluses and minuses. What may work in one project, may not work in other, because the tradeoffs that are made differ from project to project. What is a critical plus for some teams in some contexts, may not be useful for others. What is not a big minus in some cases may prove to ruin projects in other contexts.

The more implementations of a pattern you see, the more design ideas you will get when you need to implement it for your current case. The more projects you’ve done where such patterns were used, the easier and faster it will be to implement it again for a new context or to evaluate possible implementations.

 


many implementations of data access are discussed in detail in my Code Design Training

 


Featured image credit: 1tjf via 123RF Stock Photo
Posted in .NET, Design, Technical | Tagged , , , , , , , , | Leave a comment

Separating Data Access Concern

Encapsulated Data Access ImplementationIn our days most of the applications that have a relational database as storage, use an ORM to access the data. The ORM (Entity Framework, Hibernate, etc.) does most of the data access implementation. Many of them have a modern API for querying data and for creating sessions of editing and saving changes. They also provide mechanisms to hook on events of data being loaded or before data is saved.

However, using the ORM API directly into the classes which implement the business logic (and business flows) or into the Controllers (or ViewModels) which have the UI logic is not a good idea in most of the cases. It would lead to low maintainability and high costs of change. The main reasons for this are poor consistency on how the data access is done throughout the entire app and mixture of business logic with data access concerns.

Think that we are going to use EF in the entire application to access the database. In a large code base there will be places where the EF context is created in the constructor of the class that uses it, and there will be places where the context will be created in the method that needs to get or alter some data. In some cases the context will be disposed by the class that created it and in others it will not be disposed at all. There will be cases where it is passed in as a parameter by the caller code because of the entities which are bound to it. There will also be cases where under certain circumstances entities are going to be attached to newly created contexts. Does this sound familiar?

All these signal poor consistency which leads to increased complexity. When we add error handling, logging or multiple contexts for multiple databases the complexity increases exponentially and becomes uncontrollable. Adding features like: data auditing, data localization, instrumentation (for performance measurements) or enhancing the data access capabilities in any other ways, becomes very costly. It implies going through the entire code base where EF context was used and do these changes. When business logic is not well separated from data consistency validations, enhancing data access capabilities will most likely affect the use case functionalities. We’ll introduce bugs. Our code will smell as rigid and fragile.

To avoid all the above, we can abstract the data access and encapsulate its implementation by hiding the EF from the caller code. The caller code will not be able to do data access in any other ways, but the ones defined by our abstraction. The abstraction has to be good enough not to limit the capabilities of the underlying ORM, and to allow the implementation to hide it, without leaking any dependencies to above layers.

In the rest of the post I will detail such an implementation. Its code source is available on iQuarc github account, in the DataAccess repository. It is designed as a reusable component, which can be installed as a NuGet package.

The main goals of this library are:

  • to support a consistent development model for reading or modifying data in the entire application
  • to enforce separation of concerns by separating data access from business logic

One of the main ideas is that the assemblies that need to do data access (the ones that implement the business logic) are not allowed to have any references to the Entity Framework assemblies. They can only depend and use the public interface exposed by the DataAccess assembly. They will reference things like System.Data or System.Core and will take full advantage of LINQ or IQueryable, but they do not know that EF is behind. As far as they are concerned any ORM that gives a compatible IQueryProvider implementation may be used as implementation of the abstract DataAccess interfaces they use. The figure below illustrates this:

DataAccess diagram

This enforces consistency on how data access is done in the entire application. Any class that needs to access data has to use the DataAccess interfaces, because they can no longer create or get an EF context. Now, each time we need to change or enhance the data access implementation, there is one central place to do this.

Another important aspect revealed by above diagram is that the database model classes are separated into their own assembly (DataModel). Since with the code first support, we can generated simple POCOs, which have no dependencies to the EF, as classes mapped to the database tables. These POCOs remain as simple as they are, and they change only when the database model changes. They will not have logic.

Having this plus the constraint that the DataAccess assembly cannot reference the DataModel assembly we assure that the business logic does not get mixed with the data access concerns. Inside the DataAccess we cannot write any business logic (not even validations), because it does not know about domain entities and in the other assemblies we cannot have data access concerns because they are well encapsulated into the DataAccess assembly.

Now, let’s explore in more detail the code.

IRepository and IUnitOfWork are the main interfaces of the public API the DataAccess library offers. Besides them, there are few more types, but these give the development patterns of doing data access.

IRepository is meant to read data. It is a generic interface that supports queries starting from any entity.

 /// <summary>
 /// Generic repository contract for database read operations.
 /// </summary>
 public interface IRepository
 {
  /// <summary>
  /// Gets the entities from the database.
  /// </summary>
  /// <typeparam name="T">The type of the entities to be retrieved from the database.</typeparam>
  /// <returns>A <see cref="IQueryable" /> for the entities from the database.</returns>
  IQueryable<T> GetEntities<T>() where T : class;

  /// <summary>
  /// Creates a new unit of work.
  /// </summary>
  /// <returns></returns>
  IUnitOfWork CreateUnitOfWork();
 }

In most of the cases it can be received through dependency injection into the constructor of a class that needs to deal with data. Because its implementation is light (entities are for read only), its scope may be larger and its implementation can be disposed when the operation ends (see my Disposable Instances Series blog posts on how to handle the IDisposable with Dependency Injection).

The examples below, show the code patterns for reading data:

private readonly IRepository rep; // injected w/ Dependency Injection
public IEnumerable<Order> GetAllLargeOrders(int amount)
{
 var orders = rep.GetEntities<Order>()
                 .Where(o => o.OrderLines.Any(ol => ol.Ammount > amount)
 return orders.ToList();
}

Queries may be reused and returned to be enhanced or composed by the caller code, in the same operation scope:

private readonly IRepository rep; // injected w/ Dependency Injection
private IQueriable<Order> GetAllLargeOrders(int amount)
{
 var orders = rep.GetEntities<Order>()
                 .Where(o => o.OrderLines.Any(ol => ol.Ammount > amount)
 return orders;
}

public IEnumerable<OrderSummary> GetRecentLargeOrders(int amount)
{
 int thisYear = DateTime.UtcNow.Year;
 var orders = GetAllLargeOrders(amount)
                     .Where(o.Year == thisYear)
                     .Select(o => new OrderSummary
                                     {
                                      ...
                                     });
 return orders;
}

The IUnitOfWork interface is used to modify data.

 /// <summary>
 /// A unit of work that allows to modify and save entities in the database
 /// </summary>
 public interface IUnitOfWork : IRepository, IDisposable
 {
 /// <summary>
 /// Saves the changes that were done on the entities on the current unit of work
 /// </summary>
 void SaveChanges();

 /// <summary>
 /// Saves the changes that were done on the entities on the current unit of work
 /// </summary>
 Task SaveChangesAsync();

 /// <summary>
 /// Adds to the current unit of work a new entity of type T
 /// </summary>
 /// <typeparam name="T">Entity type</typeparam>
 /// <param name="entity">The entity to be added</param>
 void Add<T>(T entity) where T : class;

 /// <summary>
 /// Deletes from the current unit of work an entity of type T
 /// </summary>
 /// <typeparam name="T">Entity type</typeparam>
 /// <param name="entity">The entity to be deleted</param>
 void Delete<T>(T entity) where T : class;

 /// <summary>
 /// Begins a TransactionScope with specified isolation level
 /// </summary>
 void BeginTransactionScope(SimplifiedIsolationLevel isolationLevel);
 }

The pattern here is: read data, modify it and save the changes. These operations should be close one to the other, in a short and well defined scope, like below:

public void ReviewLargeAmountOrders(int amount, ReviewData data)
{
 using (IUnitOfWork uof = rep.CreateUnitOfWork())
 {
   IQueryable<Order> orders = uof.GetEntities<Order>()
                                 .Where(o => o.OrderLines.Any(ol => ol.Ammount > amount);
   foreach(var order in orders)
   {
     order.Status = Status.Reviewed;
     order.Cutomer.Name = data.CustomerNameUpdate;
     ...
   }

   ReviewEvent re = new ReviewEvent {...}
   uof.Add(re)

   uof.SaveCanges();
 }
}

We want the IUnitOfWork to always be used inside a using statement. To enforce encourage this, we do not register its implementation into the Dependency Injection Container, but we make a factory. The factory may be the IRepository itself. We prefer this, rather than forcing a class, that needs data, to have an extra dependency to an IUnitOfWorkFactory.

Another thing to notice here is that the IUnitOfWork inherits from IRepository. This gives two advantages:

  • queries defined for the repository may be reused on an IUnitOfWork instance to get data for changing it
  • the IUnitOfWork instance may be passed as an IRepository param to code that should only read data in the same editing context

The classes that implement these two interfaces are not part of the public API of the DataAccess. The code from above layers should not use them. To enforce this they may be made internal, which I do when the DataAccess is a project in my VS solution. On the other hand, if it is a reusable library this may be too restrictive and may not work with some Dependency Injection frameworks. Leaving them public requires some development discipline to make sure that they are not newed up in the client code.

 Another important part of the public API are the IEntityInterceptor interfaces. They give the extensibility to run custom logic at specific moments when entities are being loaded, saved or deleted.

/// <summary>
 /// Defines a global entity interceptor.
 /// Any implementation registered into the Service Locator container with this interface as contract will be applied to
 /// all entities of any type
 /// </summary>
 public interface IEntityInterceptor
 {
 /// <summary>
 /// Logic to execute after the entity was read from the database
 /// </summary>
 /// <param name="entry">The entry that was read</param>
 /// <param name="repository">A reference to the repository that read this entry. It may be used to read additional data.</param>
 void OnLoad(IEntityEntry entry, IRepository repository);

 /// <summary>
 /// Logic to execute before the entity is written into the database. This runs in the same transaction with the Save
 /// operation.
 /// This applies to Add, Update or Insert operation
 /// </summary>
 /// <param name="entry">The entity being saved</param>
 /// <param name="repository">A reference to the repository that read this entry. It may be used to read additional data.</param>
 void OnSave(IEntityEntry entry, IRepository repository);

 /// <summary>
 /// Logic to execute before the entity is deleted the database. This runs in the same transaction with the Save
 /// operation.
 /// </summary>
 /// <param name="entry">The entity being deleted</param>
 /// <param name="repository">A reference to the repository that read this entry. It may be used to read additional data.</param>
 void OnDelete(IEntityEntry entry, IRepository repository);
 }

There are global interceptors, which are triggered for all the entities of any type and there are specific entity interceptors, which are triggered for the all entities of one specific type. Interceptors are a good candidate to implement data consistency validations or data auditing. Their implementations belong to above layers and are a key element for keeping business logic outside of the data access assembly.

Another element to mention is the custom exception classes. To prevent breaking encapsulation at error handing, the DataAccess implementation wraps all the EF exceptions, for the errors that need to be passed to the client code, into custom exceptions that are defined. It abstracts the errors and lets the client code to do the error handing without depending on EF specifics.

You can explore further the DataAcess code on its GitHub repository. To see more examples on how it is used, you can look over the sample from my Code Design training, which are also available as a GitHub repository.

 


this implementation is discussed in detail in my Code Design Training

 


Featured image credit: kirillm via 123RF Stock Photo
Posted in .NET, Design, Technical | Tagged , , , , , , , , , , | Leave a comment

Disposable Instances Series

10778348_s

In the past few weeks I have published a set of four posts that deal with disposable instances. These posts describe in detail a working implementation that automatically disposes all the instances that are no longer needed, in a deterministic way. This solution works when we use Dependency Injection or Service Locator to create instances and we want to prevent, by design, leaks of expensive resources.

The posts started from a discussion on how to deal with a repository implementation, which is disposable by nature, and it evolved into a broader one, because the same challenges and approaches apply to any disposable type.

Even if I didn’t intend them from the beginning to be a series, these posts continue each other and describe the design of a component that may be part of the Application Software Infrastructure of any complex app. They include the problem description, the challenges, the different approaches which may be considered and implementation examples.

The posts are:


many solutions and discussions like the above are included in my Code Design Training

 


Image by: eteimaging / 123RF Stock Photo
Posted in .NET, Design, Technical | Tagged , , , , | Leave a comment

Disposing Instances when Using Inversion of Control

Disposable Operations

In the last few posts I have written about how to deal with IDisposable instances when using Dependency Injection. In the Who Disposes Your Repository I talk about the possibilities and challenges of disposing a repository which is injected. Then in the Extending Unity Container for IDisposable Instances (part 1 and part 2) I show how automatic dispose of all IDisposable instances can be achieved with Unity Container. This post completes the solution by detailing when the Container Hierarchies (aka Scoped Containers) are built and how they work with a Service Locator.

What we want to achieve is that all the IDisposable instances are disposed when no longer needed. Making this more specific, we want that all the IDisposable instances created within one defined scope to be disposed when that scope ends. In C# we have language support for this. We can use the using statement like this:

using (IDisposable o = new DisposableClass())
{ // scope begins

 // do stuff with o within this scope

} // scope ends

Inside the brackets of this using statement we can use the IDisposable instance as we want and we are assured that it will get disposed when the scope ends.

We can use the using statement only if:

  1. the begin and the end of the scope is in the control of our code (we write the code that defines the scope)
  2. the creation of the IDisposable instance is in the control of our code (we write the code that calls its constructor)

By doing Inversion of Control we give the control of the above to the frameworks we are using.

When we are using Dependency Injection, we no longer call the constructors. A framework (Unity Container maybe) will call it for us. However, we can still dispose instances, if we put the container itself in the using statement:

using (var scopeContainer = mainContainer.CreateChild())
{ // scope begins

 // do stuff with o within this scope
 // all the instance created within this scope are created by the scopeContainer

} // scope ends

Mainly we create one container for each scope and we dispose it when the scope ends. When the scoped container gets disposed, it will dispose all the IDisposable instances that it created (previous posts show how this can be done with Unity Container). This is how the idea of using Container Hierarchies for disposing instances came into place. Therefore, if we leave to the framework the control of building instances, we also expect the framework to dispose the instances it created. We still need to make sure that when the scope begins a new container is associated with it, and within that scope all the instances are built with the associated container.

When we are in a web application, we are not in the control of defining the scope neither (Inversion of Control again). We might want to consider this scope to be the same with handling a web request or with a web session. For example, we would want to nest all the code that handles a request within a using statement as we did above. Something quite similar would be: when the request begins create the scoped container, then keep it on the request so all the code running on that request could use it to get new instances, and when the request ends dispose it. Again, if the framework is in control of defining the scope we are interesting in, the framework should give us some hooks which we can use to run some code when the scope begins or ends and to give us some object that represents a context for this scope, so we can keep a reference to the scoped container on it.

Most of the web frameworks give such hooks. We are signaled when a request or session begins or ends. There is also an object which is easy to access, which represents the current request or session and on which we can store context information. If we are using ASP.NET MVC, which is designed with Dependency Injection in mind, we can get this done quite easy. Below is a code snippet from DevTrends github repository which contains small projects that integrate Unity Container with ASP.NET MVC and ASP.NET Web API.

public class UnityDependencyResolver : IDependencyResolver
{
  private readonly IUnityContainer _container;

  public UnityDependencyResolver(IUnityContainer container)
  {
   _container = container;
  }

  public object GetService(Type serviceType)
  {
    if (typeof(IController).IsAssignableFrom(serviceType))
    {
       return ChildContainer.Resolve(serviceType);
    }
    return IsRegistered(serviceType) ? ChildContainer.Resolve(serviceType) : null;
  }

  protected IUnityContainer ChildContainer
  {
    get
    {
      var scopeContainer = HttpContext.Current.Items[HttpContextKey] as IUnityContainer;

      if (scopeContainer == null)
         HttpContext.Current.Items[HttpContextKey] = scopeContainer = _container.CreateChildContainer();

      return scopeContainer;
    }
  }
...
}  

As you can see here the child container is created first time the framework needs it and then it is recorded on the request object.

When the Service Locator is used it is important that the scoped container gets called whenever a new instance is needed. If the container is used directly, it means that all the code that needs to request a new instance has to be able to go to the current request object, obtain the reference to the scoped container and ask the instance. If we are in an ASP.NET app this is easier because we can use the DependencyResolver.Current, which implements Service Locator pattern and which with the above integration code will go to the Unity Container recorded on the current request. If we are using another implementation which wraps a dependency container like Common Service Locator does, you will need to set it up in a way that it uses the current container. The snippet below shows an example for the Common Service Locator.

...
private void ConfigureDependencyContainer()
{
   Microsoft.Practices.ServiceLocation.ServiceLocator.SetLocatorProvider(() =>
 {
   var scopeContainer = HttpContext.Current.Items[HttpContextKey] as IUnityContainer;

   if (scopeContainer == null)
       HttpContext.Current.Items[HttpContextKey] = scopeContainer = _container.CreateChildContainer();

   return scopeContainer;
 });
}
...

All the above work well when we are in a web application, but how can we do the same in a context where we do not have a request object (given by a framework) on which we can keep the scoped container? What if we want to scope our context to a custom operation? How can we make sure that whenever ServiceLocator.Current is called from any class or any function in any thread, it will wrap over the current scoped container if the calling code is within an operation or it will go to the main container if it is outside of any operation? Such application examples may be:

  • a windows service which listens on a TCP-IP socket and handles concurrently all the commands that come on the socket. The custom operation would be all the code that handles such a command.
  • a console application which executes commands received through inline parameters. Here each command implementation would be part of the custom operation to which we would like to scope a child container.
  • a desktop application, where we would like that all the code behind a screen to be part of an operation to dispose all the instances used while that screen was opened or used.

In all these cases we can create the scoped container and put it in a using statement that nests all the code within that operation. The difficulty comes in associating the scoped container with the operation. Where to keep it so all the code that runs within that operation uses it to build new instances. We need an object (like the request in the web app) which can keep this reference. That object should be available (through a static call) to any class or function on any thread which is within the operation. In short ServiceLocator.Current needs to wrap the scoped container of the current operation.

We can implement this by creating an OperationContext class, which represents the context information of a custom operation. When it is built, it creates and stores a scoped container and it makes it available through a static getter. Here is a code snippet of this class

public class OperationContext : IDisposable
{
 private readonly IDependencyContainer container;

 private OperationContext()
 {
   container = ContextManager.GlobalContainer.CreateChildContainer();
 }

 public IServiceLocator ServiceLocator
 {
   get { return container.AsServiceLocator; }
 }

 public IDictionary Items
 {
   get
   {
     if (items == null)
       items = new Hashtable();
     return items;
   }
 }

 public void Dispose()
 {
   DisposeItems();

   IDisposable c = container as IDisposable;
   if (c != null)
     c.Dispose();
 }

 public static OperationContext Current
 {
   get { return ContextManager.Current; }
 }

 public static OperationContext CreateNew()
 {
   OperationContext operationContext = new OperationContext();
   ContextManager.SwitchContext(operationContext);
   return operationContext;
 }
...
}

Our code which defines the scope will create a new OperationContext when the operation starts and it will dispose it when the scope ends. We can do this with an using statement. The OperationContext.Current gives access to it. It can be called from any class, function on any thread and it gives the current operation. ServiceLocator.Current can wrap OperationContext.Current.ServiceLocator and all existent code which we are nesting within this using statement shouldn’t be modified. This class makes sure that the current operation context information is thread static, but is it passed to new threads which are created within current operation. It also assures that when the operation ends all the disposables it holds (including the dependency injection container) are disposed.

The OperationContext class implementation is inspired from the HttpContext class. It uses a ContextManager static class to manage the access to the storage of the context. The context store is abstracted by IContextStore interface. Its implementation has to provide thread static storage for more operations that may exist simultaneous. When we are in a console application or in a windows service, its implementation is based on the CallContext class. This assures that the context is passed along on any function call, and also to new threads which may be created from current one.

Having this, we can now define custom operations in any application in a easy way:

using(OperationContext.CreateNew())
{ //scope begins 

  // code that implements the operation
  // ServiceLocator.Current wraps the scoped container created for this operation. 

} // scope ends. OperationContext and all its content are disposed

The OperationContext is an abstract solution for giving context information to any operation regardless of the type of application. When used in a web application the IContextStore may be implemented over the HttpContext.Current and the ASP.NET remains in control of managing the context of our operation (web request).

// IContextStore implementation for an ASP.NET app
public class HttpRequestContextStore : IContextStore
{
 public object GetContext(string key)
 {
   return HttpContext.Current.Items[key];
 }

 public void SetContext(object context, string key)
 {
   HttpContext.Current.Items[key] = context;
 }
}

// setup at app startup (Global.asax.cs)
 protected void Application_Start()
 {
   ...
   ContextManager.SetContextStore(new HttpRequestContextStore());
 }

// bind OperationContext with a web request
class RequestLifetimeHttpModule : IHttpModule
{
 public void Init(HttpApplication context)
 {
   context.BeginRequest += (sender, args) => OnBeginRequest();
   context.EndRequest += (sender, e) => OnEndRequest();
 }

 private void OnBeginRequest()
 {
   OperationContext.CreateNew();
 }

 private void OnEndRequest()
 {
   OperationContext.Current.Dispose();
 }
...
}

An integrated implementation of the OperationContext, class can be found on iQuarc github repository for the AppBoot library. The OperationContext there can be used in the same way in any .NET app. An isolated sample of the OperationContext implementation code can be downloaded here:

To summarize, OperationContext class gives us the means to easily achieve what we wanted: all the IDisposable instances created withing one defined scope to be disposed when the operation ends. It does this by using scoped dependency containers which are bound to the defined scope and are disposed when the scope ends. It also gives an abstract way to easily define such a scope when our code is in control or to bind it to one created and managed by a framework.


many examples and discussions like the above are included in my Code Design Training

Posted in .NET, Design, Technical, Training | Tagged , , , , | Leave a comment

E-mail Course: Creating a Blog

I have enrolled in John’s Sonmez free e-mail course titled “How to Create a Blog That Boosts Your Career”.

I’ve joined this because I wanted to focus more on improving my blog, and because I care about John’s advice on how to do this. I was already acting on some of the advice he gave when talking about how developers can promote their selves or how they can improve their careers. I’ve picked these from his blog (simpleprogrammer.com), when he was on .NET Rocks! or in other of his materials that I get by following him on twitter.

I was also curious on how an e-mail course would go. Each lesson is in an e-mail that we (the trainees) receive twice a week. One on Monday, and one on Thursday. The lessons are quite short, few minutes to read them. Each speak about a key aspect to care and think about when building a blog. I see it like developing a set of habits that can lead you towards a fulfilling blog. Each lesson also has a short homework, which in most of the cases we should e-mail back. The nice reward I’ve gotten by doing my homework was the feedback that I’ve received from John. So far, he replied with some extra advice specific to my context to each of my homework e-mails. It makes me feel that this is not just a set of articles sent by e-mail, but it is an actual course with interaction with the trainer over the e-mail. I think it is one of the key factors of success in course ran by e-mail.

One of the things that it made me think hard about, was how to focus my posts on a topic. Which is the theme of by blog? Conclusion: Quality Code Design. The key reason I’ve started a blog in the first place was to share from my experiences. I was lucky enough to be part of many software development projects, in different roles. I’ve been dealing with difficult contexts both from technical and corporate politics perspective. There are many learnings I’ve taken along the way. One of the believes I’ve formed is that the way we organize our code, the way we design the structures in which we place the code, the way we do code design matters a lot in achieving the needed flexibility to accommodate changes in the success factors of a project (requirements, time and budget). I consider a code design to be of good quality if it stands the test of time. If it can be changed at low costs. Therefore, the conclusion I’ve reached is that I will try to focus my writings in this area. Not as stories from the projects I did in the past, but rather experiences or advices that I pick from my current work. Learnings I find worth sharing from the training and coaching I do in different companies, from the consulting and the development I do as part of iQuarc and from the work I do as a Software Architect at ISDC. And all, focused on achieving a quality code design.

At this point the course reached its second half. I can’t say I have found many new things, so far. However, it outlined some important aspects and made me think harder of them. The blog theme is one example. Another great benefit for me is that I get a strong confirmation on the practices I was already following. For me this is a good encouragement that I am going on a good path, especially when it comes from someone who is successful in this.

In the lessons to come I hope to get some guidance on aspects like:

  • How to put links from a post. Should all the words that may point to more details, have a link?
  • How to find and chose good images for the posts. Should all the posts be accompanied by a featured image?
  • How long an article should be? When is it better to finish a subject in one post and when to split it over more posts?
  • When is it better to put source code into the post and when is it better to give it as a downloadable zip.
  • Which are the pros and cons of hosting your blog under a company domain (blog.iquarc.com/florin) or come up with a name for the blog and host it on its own domain (blogname.com)

For me it was a good choice to enroll. It cost me just some time and I’ve got good advice and strong confirmation in return. A great deal. If you’d want to do the course I think you can still enroll (for a future session maybe) or you could probably get the lessons and go through them at your own pace.

Posted in Personal | Tagged , | 2 Comments

Extending Unity Container for IDisposable Instances (2nd approach)

In my previous blog post I detailed an approach of making the Unity Dependency Injection Container to automatically call Dispose() on all the IDisposable instances it builds and injects. The implementation described there, makes use of custom lifetime managers and it works fine for most of the cases except for the PerResolveLifetimeManager. Unity was extended with the PerResolve functionality in a tricky way, which makes my previous IDisposable implementation not to work for it. Therefore, I’ve came up with another solution which works in all cases. I will go into details about it here. Both these blog posts are a continuation of the ‘Who Disposes Your Repository’ post, I’ve written a while ago, where I describe a context where such an automatically disposing mechanism is desired.

For both implementations we consider that a child container is created and associated with each new scoped operation. A scoped operation may be a request or a session in a web application, or it may be a window or a view in a desktop one. During that operation the child container will be used to inject the dependencies. This is also known as using Container Hierarchies or Scoped Containers. When the operation ends, it will dispose the child container, and what we want is that this also calls Dispose() on all the IDisposable instances that were created within that operation.

Core Solution

In the previous implementation, we were recording all the IDisposable instances that are created on the lifetime manager instance associated with a type registration. Now, because this does not work for the PerResolve case (where new PerResolveLifetimeManager instances are constructed during build), we need to come with another place where to record these instances. This new place also needs to be signaled when the owning container gets disposed, so we can dispose them.

The next thing to look at is to use a custom Unity builder strategy. It is one of the most powerful mechanisms that can be used to extend the build mechanism in Unity. To create one, we need to inherit from the BuilderStrategy base class and to override one of its methods to add the code we want to be executed when any new instance gets built or torn down. Here is the code of the base class:

 /// <summary>
 /// Represents a strategy in the chain of responsibility.
 /// Strategies are required to support both BuildUp and TearDown.
 /// </summary>
 public abstract class BuilderStrategy : IBuilderStrategy
 {
 /// <summary>
 /// Called during the chain of responsibility for a build operation. The
 /// PreBuildUp method is called when the chain is being executed in the
 /// forward direction.
 /// </summary>
 /// <param name="context">Context of the build operation.</param>
 public virtual void PreBuildUp(IBuilderContext context)
 {
 }

 /// <summary>
 /// Called during the chain of responsibility for a build operation. The
 /// PostBuildUp method is called when the chain has finished the PreBuildUp
 /// phase and executes in reverse order from the PreBuildUp calls.
 /// </summary>
 /// <param name="context">Context of the build operation.</param>
 public virtual void PostBuildUp(IBuilderContext context)
 {
 }

 /// <summary>
 /// Called during the chain of responsibility for a teardown operation. The
 /// PreTearDown method is called when the chain is being executed in the
 /// forward direction.
 /// </summary>
 /// <param name="context">Context of the teardown operation.</param>
 public virtual void PreTearDown(IBuilderContext context)
 {
 }

 /// <summary>
 /// Called during the chain of responsibility for a teardown operation. The
 /// PostTearDown method is called when the chain has finished the PreTearDown
 /// phase and executes in reverse order from the PreTearDown calls.
 /// </summary>
 /// <param name="context">Context of the teardown operation.</param>
 public virtual void PostTearDown(IBuilderContext context)
 {
 }
}

For our case, we can override the PostBuildUp() and record all the IDisposable instances that get constructed. In comparison with the previous implementation, here we would keep on the strategy object all the IDisposable instances of different types (all types that implement IDisposable). There, on the lifetime manager we were keeping references only to the instances of the type registered with that lifetime manager object.

The next step is to trigger the dispose, down from the container to the recorded IDisposable instances. In its Dispose() the container disposes two things: the lifetime managers, and the extensions. See below

 protected virtual void Dispose(bool disposing)
 {
   if (disposing)
   {
     if (lifetimeContainer != null)
     {
       lifetimeContainer.Dispose();
       lifetimeContainer = null;

       if (parent != null && parent.lifetimeContainer != null)
       {
         parent.lifetimeContainer.Remove(this);
       }
     }

    // this will trigger the Dispose() into our strategy object
    extensions.OfType<IDisposable>().ForEach(ex => ex.Dispose());
    extensions.Clear();
   }
 }

The extensions dispose is our trigger point. We hold the IDisposable instances on the builder strategy object, not on the extension. To have them disposed, we can make the strategy object to implement IDisposable. There we can dispose all the instances. Then, going upwards, we can also make the custom extension that adds the strategy to the build strategies chain, to be IDisposable. It may keep a reference to the strategy and when it gets disposed by the container, it can dispose the strategy, which in its turn will dispose all the IDisposable instances that it records on the PostBuildUp(). So, when we put all the things together the code is like below

public class DisposeExtension : UnityContainerExtension,
                                IDisposable
{
  private DisposeStrategy strategy = new DisposeStrategy();

  protected override void Initialize()
  {
    Context.Strategies.Add(strategy, UnityBuildStage.TypeMapping);
  }

  public void Dispose()
  {
    strategy.Dispose();
    strategy = null;
  }

  class DisposeStrategy : BuilderStrategy, IDisposable
  {
    private DisposablesObjectList disposables = new DisposablesObjectList();

    public override void PostBuildUp(IBuilderContext context)
    {
      if (context != null)
      {
        IDisposable instance = context.Existing as IDisposable;
        if (instance != null)
        {
          disposables.Add(instance);
        }
      }

      base.PostBuildUp(context);
    }
     ...
    public void Dispose()
    {
      disposables.Dispose();
      disposables = null;
    }
  }
}

Having this done, if we add the extension to the child container after it is created, things work. All the IDisposable instances get disposed when the child container is disposed. The code snippet below shows the usage case:


// code in the application start-up area
UnityContainer container = new UnityContainer();
DisposeExtension disposeExtension = new DisposeExtension();
container.AddExtension(disposeExtension);

container.RegisterType<IService1, Service1>(new PerResolveLifetimeManager());
container.RegisterType<IService2, Service2>(new PerResolveLifetimeManager());

// some instance created out of the scoped operation
outerScopeSrv = container.Resolve<IService1>();

// code in the area that marks and new operation. It creates a child container
using (IUnityContainer childContainer = container.CreateChildContainer())
 {
   var childDisposeExt = new DisposeExtension();
   childContainer.AddExtension(childDisposeExt);

   // instances built within the operation
   scopedSrv1 = childContainer.Resolve<IService1>();
   scopedSrv2 = childContainer.Resolve<IService2>();
   ...

 } // end of the operation -> childContainer.Dispose()

AssertIsDisposed(scopedSrv1);     // -> Pass
AssertIsDisposed(scopedSrv2);     // -> Pass
AssertNotDisposed(outerScopeSrv); // -> Pass

// at application end
container.Dispose();

AssertIsDisposed(outerScopeSrv);  // -> Pass

However, there are two more aspects that need to be taken care of.

Issues

The first is about singletons. What happens when a singleton which is IDisposable gets injected through the child container? Now, it will be disposed by the first child container that used it when it gets disposed. From now on, all the other parts of the application will use the already disposed singleton instance. This is certainly a behavior we wouldn’t want. If a singleton is IDisposable (it’s another discussion why would we do that in the first place) it should not be disposed in the first operation that uses it. The code snippet below shows this case.

UnityContainer container = NewContainer();

// register as singleton
container.RegisterType<IService1, Service1>(new ContainerControlledLifetimeManager());

IService1 singletonSrv = container.Resolve<IService1>();
IService1 scopedSrv;

using (IUnityContainer childContainer = CreateChildContainer(container))
{
    scopedSrv = childContainer.Resolve<IService1>();
    Assert.AreSame(s, scopedService);
}

AssertNotDisposed(singletonSrv); // -> Fail

To fix this, inside the strategy, we need to verify if the instance that is being built is singleton. If yes, it should not be recorded for dispose, in the strategy that belongs to the child container. To do this check, we look if the lifetime manager is ContainerControlledLifetimeManager and if it comes from the parent container. Below is the code that does it

...
 private bool IsControlledByParrent(IBuilderContext context)
 {
 IPolicyList lifetimePolicySource;
 ILifetimePolicy activeLifetime = context.PersistentPolicies
                            .Get<ILifetimePolicy>(context.BuildKey, out lifetimePolicySource);

 return activeLifetime is ContainerControlledLifetimeManager
        && !ReferenceEquals(lifetimePolicySource, context.PersistentPolicies);
 }

This code already starts to make our extension a bit ugly. It hardcodes that the singletons are being registered with this particular lifetime manager type. If one will create a new custom lifetime manager to implement singletons, but with some additional behavior our extension may not work. Another thing that is a bit ugly, is that we need to search each time an instance is built, for the lifetime manager recursively upwards. However, I don’t see a better way to make this work for this case.

The second thing that needs to be fixed is more difficult to observe. The issue appears when we add this extension both to the parent and to the child container. This may happen either because we want the automatic dispose for the parent container too, or when we have deeper container hierarchies, meaning that we create a child container from another child container because of nested operations (probably not the case in a web app, but it could be the case in a desktop app).

By design, when a child container is created it inherits (by referencing) all the build strategies of the parent container. In general this is a good idea, because the strategies the parent container was configured with are also used by its children, and this brings consistency. However, for our case this is problematic. When the child container builds an IDisposable instance, that instance will be recorded twice: once in the child container DisposeStrategy object, and one more time in the parent DisposeStrategy object (we need to have different DisposeStrategy objects in the parent and in the child, because on these objects we record the IDisposable instances). Recording the instances built by the child container into the parent container strategy object may lead to memory leaks. The child containers are disposed once their operation ends, but the parent container will live typically as long as the application does. Its strategy object will also live with it, and it will grow with WeakRefernce objects for each instance built by all the child containers in all operations. This is bad!

It is also a sign that this approach to implement the automatic dispose, by keeping the reference on a builder strategy may not be in line with the design of Unity. The strategies were not meant to keep state from build to build. They were rather meant to extend the behavior of building instances, by adding logic that operates on the data from the IBuilderContext. However, because PerResolveLifetimeManager is implemented in a questionable way, now we are working around it with another questionable extension :(.

The fix for this case is not nice. In the PostBuildUp() method we need to determine if this strategy object belongs to the container that initiated the current build (the child) or it is an inherited strategy. If doesn’t belong to current container, we should not record currently built instance. The IBuilderContext which we get into the PostBuildUp(), does not contain any information on the container that builds this instance. So we cannot use a container match to distinguish if currently built instance should be registered in current builder strategy object or not. The only way we could distinguish between these cases is to relay on the order in which the strategies are put in the strategy chain which is constructed before a build starts. Always the strategy chain is created by placing the inherited strategies first, and at the end the specific strategies of the current container. Therefore, if the current strategy object is not the last in the strategy chain, it means that it is inherited and we should not record the current instance. We are relying on the ordering done in the StagedStrategyChain<T> class which implements the IStagedStrategyChain interface. It is not very clear if the ordering we are relying on is by design, meaning that it should be preserved in future versions or it is just an implementation detail. Therefore, this should be an attention point when new versions of Unity will be released. The code for this fix is shown in the snippet below:

private bool IsInheritedStrategy(IBuilderContext context)
{
   // unity container puts the parent container strategies before child strategies when it builds the chain
   IBuilderStrategy lastStrategy = context.Strategies
                          .LastOrDefault(s => s is DisposeStrategy);

   return !ReferenceEquals(this, lastStrategy);
 }

In the end, if we put everything in together, we get the desired dispose behavior both for PerResolveLifetimeManager and for TrainsientLifetimeManager with this approach. The entire source code that implements this approach can be downloaded from here:

Conclusion

Both of the two approaches that I’ve presented in the previous blog post and in this one, have pluses and minuses. There is a trade-off to be made when picking one over the other. The first one, which uses the lifetime managers to record the IDisposable instances has a cleaner design. It is in-line with the way Unity was meant to be extended for lifetime and dispose management. Its disadvantage is that it does not work with the PerResolveLifetimeManager.

The second approach, which uses a builder strategy to record the IDisposable instances works with all the lifetime managers, including the PerResolveLifetimeManager. Its disadvantage is that it is an ugly extension. It holds state on a builder strategy and it may rely on some of the current implementation details. These makes it vulnerable to future versions of Unity or when combining it with other extensions.

If I were to choose, I would use the first one if I do not need the PerResolveLifetimeManager. If I need it, then I would fall back to the second one and carefully test if it works with the version of Unity that I am using and with the other extensions that I need. The good part is that switching from one implementation to the other is done by changing the way the container is configured. Usually this code is separated from the rest of the application, so by doing this switch there should be little if any change in the code that implements the use cases. Therefore, a switch with small costs.


many examples like the above are included in my Code Design Training

Posted in .NET, Design, Technical, Training | Tagged , , , | Leave a comment

Extending Unity Container for IDisposable Instances (1st approach)

A few weeks ago, in my blog post ‘Who Disposes Your Repository’ I wrote about the challenges of implementing an IDisposable repository, which takes full advantage of the deferred execution of the IQueryable<T> and, which is being injected through Dependency Injection (DI) into the classes that need to read or store data. I have focused the discussion on the case when the repository is abstracted by an interface, and its implementation is injected through DI. I have detailed more alternatives on how it could be disposed and the challenges that arise from the stateful nature of a repository, the deferred execution of the IQueryable<T> and the DI.

In the end I have argued that especially for large applications, I would prefer that the Dependency Injection Container (DIC) disposes all the IDisposable instances it created and it injected, including the repository. In short, the code that creates an IDisposable instance should also be responsible to call Dispose() on it.

In this post I will discuss some of the challenges of achieving this with Unity Container. By working to extend Unity with this functionality I have ended up implementing two approaches. In this post I will detail the first implementation I did, and in a next post I’ll explain the other.

There are many other writings on this. For example I have found useful the posts of Rory Primrose and of Marcel Veldhuizen who describe different approaches for disposing instances created by Unity. They both describe very well how Unity works and how it can be extended for this purpose. However, I was looking for an approach in which Unity remains unknown to the client code. I didn’t want the client code to call Teardown() or other Unity specific methods.

I wanted that when an operation ends all the IDisposable instances that were created and injected by Unity within that operation to be automatically disposed. Such an operation may be a request or a session in a web application, or may be a window or a view in a desktop one. In general any well-defined scope in any application.

My approach is based on using the Container Hierarchies (also known as Scoped Containers). Unity, as many other dependency containers, supports this. When you call CreateChildContainer() method, it will create a container that inherits all the configurations form the current container. The idea is that when an operation begins, a child container will be created and associated with it. During that operation all the dependency injection will be done using the child container. This means that all the new instances injected during the operation, will be created by the child container. When the operation ends, the child container will be disposed. The Dispose() of the child container should trigger the Dispose() for all IDisposable instances that were created.

Here is a good example on how to associate a child container with a web request on an ASP.NET MVC app or here for a WebApi app. I will come back in a future post on this, to show how this association can be done in a more abstract way which can also work when we are not in a web application.  Now, I will dive into how to extend to trigger the Dispose() from the child container to all the IDisposable instances that it created.

The problem to solve boils down to these:

  • Keep weak references to all the IDisposable instances that were created by the container (the child container)
  • Make sure that the Dispose() function of Unity will also call Dispose() on the instances we are referencing

The first thing I tried, was to make use of the lifetime managers. Unity defines this as an extension point to allow external code to control how references to object instances are stored and how the container disposes these instances. For example they are used to implement Singleton-like instances. When the container is configured, using RegisterType() method, an instance of a LifetimeManager has to be also given. The container will keep as part of its configuration, all the instances of the lifetime managers associated to the configured types, and it will call them when it injects instances of those types. A lifetime manager class is in general simple. It inherits the LifetimeManager base class and by overriding GetValue() and SetValue() methods, it can control the lifetime of the instances of the type it was configured with.

For example if you want that a certain type to be Singleton like, you would use the ContainerControlledLifetimeManager like this:

container.RegisterType<IMySingletonService, MySingletonService>(
                        new ContainerControlledLifetimeManager());

If we look into the code of the ContainerControlledLifetimeManager we see that the lifetime manager keeps a reference to the instance and it will return it each time GetValue() is called (for simplicity I modified a bit the code of this class):

public class ContainerControlledLifetimeManager : LifetimeManager,                                                  IDisposable
{
 private object value;

 /// <summary>
 /// Retrieve a value from the backing store associated with this Lifetime policy.
 /// </summary>
 /// <returns>the object desired, or null if no such object is currently stored.</returns>
 public override object GetValue()
 {
   return this.value;
 }

 /// <summary>
 /// Stores the given value into backing store for retrieval later.
 /// </summary>
 /// <param name="newValue">The object being stored.</param>
 public override void SetValue(object newValue)
 {
   this.value = newValue;
 }

 /// <summary>
 /// Remove the given object from backing store.
 /// </summary>
 public override void RemoveValue()
 {
   this.Dispose();
 }

 public void Dispose()
 {
   this.Dispose(true);
   GC.SuppressFinalize(this); // shut FxCop up
 }

 protected virtual void Dispose(bool disposing)
 {
   if (this.value != null)
   {
      if (this.value is IDisposable)
      {
         ((IDisposable)this.value).Dispose();
      }
      this.value = null;
   }
 }
}

The first time an instance of MySingletonClass needs to be injected, the Unity container will first call the GetValue() of the lifetime manager. If that returns null, the container will build a new instance, will call the SetValue() of the lifetime manager to pass it back, and then it will inject it further. Next time an instance of same time is needed, the GetValue() will return the previously created one, so the container will not build a new one. Therefore, the Singleton-like behavior.

The TransientLifetimeManger is at the other end. It is used when you want new instances to be created each time they need to be injected. It is also the default. Its code is even simpler because GetValue() always returns null, which makes the container to build new instances each time.

/// <summary>
/// An <see cref="LifetimeManager"/> implementation that does nothing,
/// thus ensuring that instances are created new every time.
/// </summary>
public class TransientLifetimeManager : LifetimeManager
{
 /// <summary>
 /// Retrieve a value from the backing store associated with this Lifetime policy.
 /// </summary>
 /// <returns>the object desired, or null if no such object is currently stored.</returns>
 public override object GetValue()
 {
   return null;
 }

 /// <summary>
 /// Stores the given value into backing store for retrieval later.
 /// </summary>
 /// <param name="newValue">The object being stored.</param>
 public override void SetValue(object newValue)
 {
 }

 /// <summary>
 /// Remove the given object from backing store.
 /// </summary>
 public override void RemoveValue()
 {
 }
}

Unity design and documentation encourages using the lifetime managers to control the disposing of created instances. On its Dispose() the container will call the Dispose() on all the lifetime manager instances that implement IDisposable. However, the TransientLifetimeManger is not IDisposable. This is somehow normal because it does not keep reference to anything, so nothing to Dispose(). To achieve our goal, I have created a DisposableTransientLifetimeManger like this:

public class DisposableTransientLifetimeManager : TransientLifetimeManager,
                                                  IDisposable
{
 private DisposableObjectList list = new DisposableObjectList();

 public override void SetValue(object newValue)
 {
   base.SetValue(newValue);

   IDisposable disposable = newValue as IDisposable;
   if (disposable != null)
      list.Add(disposable);
 }

 public void Dispose()
 {
   list.Dispose(); // this will call Dispose() on all the objects from the list
 }
}

The SetValue() will populate a list which keeps weak references to all instances which are disposable. On Dispose() it will just dispose all the elements in the list. Simple. The ContainerControlledLifetimeManager is already IDisposable and it does dispose the instance it references. So now, if we use the disposable lifetime managers when we configure the container, all the instances which are IDisposable will be disposed when the container gets disposed.

This works fine when we dispose the container that is directly configured, with the disposable lifetime managers. However, what we wanted was to configure once the main container, and then to use child containers for each operation (request) and to dispose the child containers only. The child containers, would use the configuration of the parent. The code snippet below shows this usage case:


 // configuring the main container
 UnityContainer container = new UnityContainer();
 container.RegisterType<IService1, Service1>(new DisposableTransientLifetimeManager());
 container.RegisterType<IService2, Service2>(new DisposableTransientLifetimeManager());

 using (var childContainer = container.CreateChildContainer()) // child container should be associated with an operation (request)
{
   // some instances created within the operation (request)
   s11 = childContainer.Resolve<IService1>();
   s12 = childContainer.Resolve<IService1>();

   s21 = childContainer.Resolve<IService2>();
   s22 = childContainer.Resolve<IService2>();
} //childContainer.Dispose()

 AssertIsDisposed(() => s11.Disposed); //--> fail
 AssertIsDisposed(() => s12.Disposed); //--> fail

 AssertIsDisposed(() => s21.Disposed); //--> fail
 AssertIsDisposed(() => s22.Disposed); //--> fail

This does not work as we expected, because when the child container is disposed it does not have any lifetime managers instances in it. This is because it uses the configuration from the parent, including the instances of the lifetime mangers given there. This is by design in Unity. If the lifetime mangers instances from the parent would not be used, we would get into short lived singletons issues. Think that you configure a certain type to be Singleton. If you get objects of it through main container you get instance1, but if you get it through a child container you would get instance2. To prevent this, the same instance of lifetime manager (the one given at RegisterType()) is used by child containers too.

For our case, what we would like, is that for our disposable lifetime managers, the child container to create new instances of them and to use those to manage the lifetime of the objects it builds and injects. We can achieve this in by creating a custom Unity extension. Extensions are a more powerful way to extend the behavior of Unity and are often used in combination with the lifetime managers. As any powerful extension mechanism you can tweak the default behavior a lot, but when not used carefully you can work against the original design of the framework and create complexity. In our case we want to achieve the exact same thing as the HierarchicalLifetimeStrategy does for the HierarchicalLifetimeManager. So, I pretty much copied its code, into a new generic extension for any hierarchical lifetime manager, like this:

public class HierarchicalLifetimeExtension<T> : UnityContainerExtension where T : LifetimeManager, new()
{
 protected override void Initialize()
 {
   Context.Strategies.AddNew<HierarchicalLifetimeStrategy<T>>(UnityBuildStage.Lifetime);
 }

  /// <summary>
  /// A strategy that handles hierarchical lifetimes across a set of parent/child
  /// containers.
  /// </summary>
  private class HierarchicalLifetimeStrategy<T> : BuilderStrategy
 where T : LifetimeManager, new()
  {
   /// <summary>
   /// Called during the chain of responsibility for a build operation. The
   /// PreBuildUp method is called when the chain is being executed in the
   /// forward direction.
   /// </summary>
   public override void PreBuildUp(IBuilderContext context)
   {
     IPolicyList lifetimePolicySource;

     var activeLifetime = context.PersistentPolicies.Get<ILifetimePolicy>(context.BuildKey, out lifetimePolicySource);
     if (activeLifetime is T && !object.ReferenceEquals(lifetimePolicySource, context.PersistentPolicies))
     {
      // came from parent, add a new lifetime manager locally
      var newLifetime = new T();

      context.PersistentPolicies.Set<ILifetimePolicy>(newLifetime, context.BuildKey);
      context.Lifetime.Add(newLifetime);
      }
    }
  }
}

Now, if we put all the pieces together, we get the behavior we wanted. The snippet below shows the result.

 // configuring the main container
 UnityContainer container = new UnityContainer();
 container.RegisterType<IService1, Service1>(new DisposableTransientLifetimeManager());
 container.RegisterType<IService2, Service2>(new DisposableTransientLifetimeManager());

 var outterScopeSrv = container.Resolve<IService1>();

 using (var childContainer = container.CreateChildContainer()) // child container should be associated with an operation (request)
{
    // adding this extension to the child, makes the difference from previous code snippet
    childContainer.AddExtension(
         new HierarchicalLifetimeExtension<DisposableTransientLifetimeManager>());

   // some instances created within the operation (request)
   s11 = childContainer.Resolve<IService1>();
   s12 = childContainer.Resolve<IService1>();

   s21 = childContainer.Resolve<IService2>();
   s22 = childContainer.Resolve<IService2>();
} //childContainer.Dispose()

 AssertIsDisposed(() => s11.Disposed); //--> success
 AssertIsDisposed(() => s12.Disposed); //--> success

 AssertIsDisposed(() => s21.Disposed); //--> success
 AssertIsDisposed(() => s22.Disposed); //--> success

 AssertIsNotDisposed(() => outerScopeSrv.Disposed); //--> success

So, when the child container is disposed it will call the Dispose() of all the IDisposable lifetime mangers within the child container. The DisposableTransientLifetimeManager we have created and use is IDisposable and on its Dispose() it will call the Dispose() of all the IDisposable instances that it references. The HierarchicalLifetimeExtension that we have created and added to the child container, makes sure that when an instance is to be build, for a type that was configured in the parent container, a new instance of the same lifetime manager is created and added into the child container to be used from now on when building objects of that particular type.

This approach works well for transient lifetime manager and for most of the other lifetime managers, if are extended with the IDisposable implementation in the same way. It is inline with the Unity design and not difficult to understand and use.

You can download all the source code in a zip file:

However, this approach does not work for the PerResolveLifetimeManager. Yes, it was a bad surprise for me too :(. PerResolveLifetimeManager is not IDisposable, but that’s not the issue. We could make a DisposablePerResolveLifetimeManager as we did for the transient, and collect in it weak references to all IDisposable instances. However, the PerResolveLifetimeManager behavior is implemented with the DynamicMethodConstructorStrategy, and this is problematic. Under certain conditions this strategy creates new instances of the PerResolveLifetimeManager. This seems to work against the original design of Unity, because by creating more new instances of a lifetime manager within same container instance, the lifetime manager is striped by its main purpose, which is to control the lifetime of the instances of a particular type. Here, the used instance of the lifetime manager is no longer given through RegisterType() method, but it is being created during the object build. So, our current approach does not work for this lifetime manager for two main reasons: not all of the new instances of the PerResolveLifetimeManager get stored into the container to be disposed, and the DynamicMethodConstructorStrategy will build new instances of PerResolveLifetimeManager not of DisposablePerResolveLifetimeManager as we’d want.

For the cases when we would like to use both the PerResolveLifetimeManager and the TransientLifetimeManger I ended up making another implementation to extend Unity with same behavior of automatically disposing all the IDisposable instances, when the child container gets disposed. I will detail it in a future post.


many examples like the above are included in my Code Design Training

Posted in .NET, Design, Technical, Training | Tagged , , , | Leave a comment

Low Coupling by Refactoring Towards Higher Cohesion

Low coupling and high cohesion go hand in hand. In a low coupled design, the modules, classes and functions have a high cohesion. The other way around is also true: making high cohesive modules, classes or functions leads to a loosely coupled design.

Why is this useful or interesting? The thing is that in designing software we never get directly to a good design. We can’t make a good design from the start. We improve the design of our code by gradually refactoring it. We put into the code the knowledge we learn by experimenting with it. This is code refactoring. Therefore, I think it is important to identify and to know patterns of refactoring, in order to increase our efficiency in getting to a good design. Refactoring towards high cohesive classes and functions is a pattern of refactoring our code to result a lower coupled design. I will present it bellow by walking through an example.

This article turned out to be quite long. For easier reading I have splat it in three. The first part reviews few concepts and the second part dives into the code, walks us through the small refactoring steps and explains the reasoning behind. At the end it summaries the refactoring pattern steps and a few smells to pay attention to.

Part 1: reviewing few concepts

Coupling refers to how tight the connection is between two elements of our system. Lower the coupling is the better isolated the elements of our system are. This means that changes in one element would not trigger changes in the others. Generally, the looser the coupling is the better the design is because it is less resistant to changes. However, some coupling is needed. The system elements do need to interact, and in a loosely coupled design this interaction is well defined through abstractions. The inner details of an element (module or class) are well encapsulated (hidden) and are not relevant when thinking on how to interact with others. When details change, they do not trigger waves of changes in all the system making it resistant to change. So coupling is tolerated where it is needed, meaning when it brings benefits and it should be done through good abstractions. In other words: Don’t tolerate coupling without benefits!

Identifying the good abstractions and implementing them through well encapsulated elements is not trivial, because we have a limited view on the system in the beginning. We couple things by nature. We put things together when we do not see the whole picture. Then when it is revealed, we need to stop, we need to refactor to break them up. Then we can identify the good abstractions which lead towards a better design.

A class has high cohesion when most of (all off) its fields are used by most of (all of) its methods. The more fields a method manipulates the more cohesive that method is to the class. A class in which each field is used by each method is maximally cohesive. When cohesion is high, the methods and fields of the class are co-dependent and hang together as a logical whole. Generally, it is neither advisable nor possible to create such maximally cohesive classes, but on the other hand we would like the cohesion to be high. For a function, cohesion refers to how closely the operations in a routine are related. The function code should consistently use all the variables it declare and all the input parameters to compute the result.

When we observe a class with poor cohesion, for example a class which has two parts, one made of three public methods which operate on two fields and another one made of two public methods which operate on other three fields, the general pattern to increase its cohesion is to split it in two classes and to make one to use the other. This way we can identify good abstractions and more code reuse opportunities, which will improve our design.

The example we will dive into shows a pattern to refactor to lower coupling, by improving the cohesion. The first step is to improve the public interface of a class by reducing the number of parameters of its functions. This reveals better the low cohesion of the class and then in successive refactoring steps we increase it, by splitting the class into more.

The pattern to reduce the number of parameters from public methods is to transform them into class fields, which are set through the constructor. This makes the class interface to be more cohesive and it reduces the complexity of the caller code. As consequence, it may also reveal the poor cohesion of the class, by now having groups of fields used by groups of methods. The refactor goes in small steps improving the design a little by little in controllable small chunks.

Part 2: refactoring example

The class that we are looking at is a class that is used to export XML files that represent pages of data about customers and their orders. The class gives functions that can export XML pages with detailed customer information or only with orders. The core functionality this class implements stays in the logic to fill in a XML with data (to simplify the example I have taken out this code)

To follow easier this refactoring example, you can download the source code file from each refactoring step from github here. We start with 00_PageXmlExport.cs, which is the initial state of the code and then when we advance you can look at 01_PageXmlExport.cs after first refactor step, then 02_PageXmlExport.cs for the second refactor step and so on until the fifth refactor step.

// file: http://tinyurl.com/00-PageXmlExport-cs
public class PageXmlExport
{
  private const string exportFolder = "c:\temp";

  public bool ExportCustomerPage(
                                 string fileNameFormat,
                                 bool overwrite,
                                 string customerName,
                                 int maxSalesOrders,
                                 bool addCustomerDetails)
  {
    string fileName = string.Format(fileNameFormat, "CustomerPage", customerName, DateTime.Now);
    string filePath = Path.Combine(exportFolder, fileName);

    if (!overwrite && File.Exists(filePath))
         return false;

    PageXml content = new PageXml {Customer = new CustomerXml
                                  {Name = customerName}};

    using (var repository = new EfRepository())
    {
       if (maxSalesOrders > 0)
       {
          var orders = repository.GetEntities<Order>()
               .Where(o => o.Customer.CompanyName == customerName)
               .OrderBy(o => o.OrderDate)
               .Take(maxSalesOrders);

          //enrich content with orders
          // ...
       }

       if (addCustomerDetails)
       {
          var customer = repository.GetEntities<Customer>()
                         .Where(c => c.CompanyName == customerName);

          // enrich content with customer data
          // ...
       }
    }

     XmlSerializer serializer = new XmlSerializer(typeof (PageXml));
     using (StreamWriter sw = File.CreateText(filePath))
     {
          serializer.Serialize(sw, content);
     } 

     return true;
   }
...
}

ExportCustomerPage(…) function will write on the disk an XML page containing customer details and customer orders, which are read from the database using a Repository implementation. It receives as input parameters a fileNameFormat with the pattern to generate the name of the file and a flag that says whether it should overwrite the file in case it already exits. It also receives the name of the customer to export the data of, and the maximum orders that should be exported. It gets an extra setting weather it should include or not the customer details.

This function evolved over time in this shape. The desire to reuse some of the code that implements the logic of enriching the XML with orders into the one that also enriches the XML with customer data, made the developers to add a the addCustomerDetails flag and correspondent code in here. Also the logic of composing the file name and/or overwriting it was added later, in the easiest way to implement a new request.

Looking further, the same desire to reuse the code that enriches the XML, made it that later a new function which includes external data into the exported XML was added to the same class. This is how ExportCustomerPageWithExternalData(…) got written. It does the same as the above, but if external data is present, then it is included in the XML.

// file: http://tinyurl.com/00-PageXmlExport-cs
public class PageXmlExport
{
...
   public bool ExportCustomerPageWithExternalData(
                               string fileNameFormat,
                               bool overwrite,
                               string customerName,
                               int maxSalesOrders,
                               bool addCustomerDetails,
                               PageData externalData,
                               ICrmService crmService,
                               ILocationService locationService)
   {
     string fileName = string.Format(fileNameFormat, "CustomerPage", customerName, DateTime.Now);
     string filePath = Path.Combine(exportFolder, fileName);

     if (!overwrite && File.Exists(filePath))
        return false;

     PageXml content = new PageXml {Customer = new CustomerXml
                                   {Name = customerName}};

     if (externalData.CustomerData != null)
     {
        // enrich content with externalData.CustomerData
        // ...
     }
     else
     {
        CustomerInfo customerData = crmService.GetCustomerInfo(
                                           content.Customer.Name);

        // enrich content with customer data
        // ...
     }

     using (EfRepository repository = new EfRepository())
     {
        if (maxSalesOrders > 0)
        {
          var orders = repository.GetEntities<Order>()
            .Where(o => o.Customer.CompanyName == content.Customer.Name)
            .OrderBy(o => o.OrderDate)
            .Take(maxSalesOrders);

          //enrich content with orders
        }

       if (addCustomerDetails)
       {
          var customer = repository.GetEntities<Customer>()
            .Where(c => c.CompanyName == customerName);

          // enrich content by merging the external customer data with what read from DB
          // ...
       }
     }

     if (locationService != null)
     {
        foreach (var address in content.Customer.Addresses)
        {
          Coordinates coordinates = locationService.GetCoordinates(address.City, address.Street, address.Number);
          if (coordinates != null)
            address.Coordinates = string.Format("{0},{1}", coordinates.Latitude, coordinates.Longitude);
        }
     }

     XmlSerializer serializer = new XmlSerializer(typeof (PageXml));
     using (StreamWriter sw = File.CreateText(filePath))
     {
        serializer.Serialize(sw, content);
     }

     return true;
   }
...
}

Following the same approach to reuse the core code that enriches the XML, other functions we added in time. ExportOrders(..) exports a XML with the same schema, but with all the orders the customer has and without additional customer data.

// file: http://tinyurl.com/00-PageXmlExport-cs
public class PageXmlExport
{
...
   public bool ExportOrders(string fileNameFormat, bool overwrite, string customerName)
   {
     string fileName = string.Format(fileNameFormat, "CustomerOrdersPage", customerName, DateTime.Now);
     string filePath = Path.Combine(exportFolder, fileName);

     if (!overwrite && File.Exists(filePath))
       return false;

     PageXml content = new PageXml {Customer = new CustomerXml {Name = customerName}};

     using (EfRepository repository = new EfRepository())
     {
       var orders = repository.GetEntities<Order>()
          .Where(o => o.Customer.CompanyName == content.Customer.Name)
          .OrderBy(o => o.ApprovedAmmount)
          .ThenBy(o => o.OrderDate);

        //enrich content with orders
      }

      XmlSerializer serializer = new XmlSerializer(typeof (PageXml));
      using (StreamWriter sw = File.CreateText(filePath))
      {
         serializer.Serialize(sw, content);
      }

      return true;
   }
...
}

Later on, because this was the class that knew how to produce XMLs with customer orders, it got two new methods: GetPagesFromOrders(…) and ExportPagesFromOrders(…), which were useful when the data should not be taken from the database, but it is given as input parameter.

// file: http://tinyurl.com/00-PageXmlExport-cs
public class PageXmlExport
{
...
   public IEnumerable<PageXml> GetPagesFromOrders(
                                IEnumerable<Order> orders,
                                int maxSalesOrders,
                                ICrmService crmService,
                                ILocationService locationService)
   {
     Dictionary<string, IEnumerable<Order>> customerOrders =
                                    GroupOrdersByCustomer(orders);
     foreach (var customerName in customerOrders.Keys)
     {
       PageXml content = new PageXml {Customer = new CustomerXml
                                     {Name = customerName}};

       if (crmService != null)
       {
          CustomerInfo customerData = crmService.GetCustomerInfo(content.Customer.Name);
          //enrich with data from crm
       }

       var recentOrders = customerOrders[customerName]
                                   .OrderBy(o => o.OrderDate)
                                   .Take(maxSalesOrders);
       foreach (var order in recentOrders)
       {
           // enrich content with orders
           // ...
       }

       if (locationService != null)
       {
         foreach (var address in content.Customer.Addresses)
         {
            Coordinates coordinates = locationService.GetCoordinates(address.City, address.Street, address.Number);
            if (coordinates != null)
              address.Coordinates = string.Format("{0},{1}", coordinates.Latitude, coordinates.Longitude);
         }
       }

       yield return content;
     }
   }

   public bool ExportPagesFromOrders(
                                 string fileNameFormat,
                                 bool overwrite,
                                 IEnumerable<Order> orders,
                                 int maxSalesOrders,
                                 ICrmService crmService,
                                 ILocationService locationService)
   {
     IEnumerable<PageXml> pages = GetPagesFromOrders(orders, maxSalesOrders, crmService, locationService);
     foreach (var pageXml in pages)
     {
        string customerName = pageXml.Customer.Name;
        string fileName = string.Format(fileNameFormat, "CustomerOrdersPage", customerName, DateTime.Now);
        string filePath = Path.Combine(exportFolder, fileName);

        if (!overwrite && File.Exists(filePath))
            return false;

        XmlSerializer serializer = new XmlSerializer(typeof (PageXml));
        using (StreamWriter sw = File.CreateText(filePath))
        {
           serializer.Serialize(sw, pageXml);
        }
     }

     return true;
   }
...
}

Now, look at the public methods signature only. They have many and redundant parameters. This makes them hard to be used by the caller code.

// file: http://tinyurl.com/00-PageXmlExport-cs
public class PageXmlExport
{
...
  public bool ExportCustomerPage(
                                 string fileNameFormat,
                                 bool overwrite,
                                 string customerName,
                                 int maxSalesOrders,
                                 bool addCustomerDetails)
  {...}

  public bool ExportCustomerPageWithExternalData(
                                 string fileNameFormat,
                                 bool overwrite,
                                 string customerName,
                                 int maxSalesOrders,
                                 bool addCustomerDetails,
                                 PageData externalData,
                                 ICrmService crmService,
                                 ILocationService locationService)
  {...}

  public bool ExportOrders(
                                string fileNameFormat,
                                bool overwrite,
                                string customerName)
  {...}

  public IEnumerable<PageXml> GetPagesFromOrders(
                                IEnumerable<Order> orders,
                                int maxSalesOrders,
                                ICrmService crmService,
                                ILocationService locationService)
  {...}

  public bool ExportPagesFromOrders(
                                string fileNameFormat,
                                bool overwrite,
                                IEnumerable<Order> orders,
                                int maxSalesOrders,
                                ICrmService crmService,
                                ILocationService locationService)
  {...}
...
}

The first step of refactor is to reduce the number of parameters of the first two methods. We move some common parameters into the constructor. It results the following code (file 01_PageXmlExport.cs).

// file: http://tinyurl.com/01-PageXmlExport-cs
public class PageXmlExport
{
  private const string exportFolder = &amp;quot;c:\temp&amp;quot;;

  private readonly string fileNameFormat; //used in 3/5 methods
  private readonly bool overwrite; //used in 3/5 methods

  private readonly int maxSalesOrders; // used in 3/5 methods
  private readonly bool addCustomerDetails; // used in 2/5 methods

  public PageXmlExport(string fileNameFormat,
                       bool overwrite,
                       int maxSalesOrders,
                       bool addCustomerDetails)
 {
    this.fileNameFormat = fileNameFormat;
    this.overwrite = overwrite;
    this.maxSalesOrders = maxSalesOrders;
    this.addCustomerDetails = addCustomerDetails;
 }

 public bool ExportCustomerPage(string customerName)
 {...}

 public bool ExportCustomerPageWithExternalData(
                                 string customerName,
                                 PageData externalData,
                                 ICrmService crmService,
                                 ILocationService locationService)
 {...}
...
}

 This already makes the functions to look better. They are easier to be called. The caller can now instantiate this class with the settings it needs, and then call ExportCustomerPage(…) for all the customer names it wants. Previously, it had to repeat most of the parameters for each customer call. If we look at the resulted fields we can see that most of them are used in 3 of the 5 functions. The rest of the code remains pretty much the same.

Continuing on this path, the next small refactor step is to reduce the number of parameters for the next functions. The resulted code is the following (file 02_PageXmlExport.cs).

// file: http://tinyurl.com/02-PageXmlExport-cs
public class PageXmlExport
{
  private const string exportFolder = &amp;quot;c:\temp&amp;quot;;
  private readonly string fileNameFormat; //used in 3/5 methods
  private readonly bool overwrite; //used in 3/5 methods

  private readonly int maxSalesOrders; // used in 3/5 methods
  private readonly bool addCustomerDetails; // used in 2/5 methods

  private readonly ICrmService crmService; // used in 3/5 methods
  private readonly ILocationService locationService; // used in 3/5 methods

  public PageXmlExport( string fileNameFormat,
                        bool overwrite,
                        int maxSalesOrders,
                        bool addCustomerDetails,
                        ICrmService crmService,
                        ILocationService locationService)
  { ... }

  public bool ExportCustomerPageWithExternalData(
                                         string customerName,
                                         PageData externalData)
  { ... }

  public bool ExportOrders(string customerName)
  { ... }

  public IEnumerable<PageXml> GetPagesFromOrders(IEnumerable<Order> orders)
  { ... }

  public bool ExportPagesFromOrders(IEnumerable<Order> orders)
  { ... }
...
}

Now looking again only at the signature of the public functions of this class. They look much better. The public interface is more consistent and this simplifies the caller code.

Analyzing further the refactor result, we see that the class has in the constructor 6 dependencies. This is quite a lot. Looking at the fields, it has 7 fields, from which 2 are not complex (ICrmService and ILocationService). Having so many fields and dependencies is a bad smell. On a closer look we may observe that some fields are used only by some functions and not all functions use all fields. This is a clear symptom of a class with poor cohesion.

To refactor towards a better cohesive class, we should identify groups of fields, used by same methods. For example, the exportFolder, fileNameFormat and overwrite are used by all functions that write to disk. Even more, they are only used in the logic that relates to writing the file. When we identify such a group of dependencies it is an opportunity to reduce the dependencies of the class by moving them into a new class, which will be used by the current one. While doing this, is good to also think about a good abstraction for the class we are going to create, an abstraction that express well the functionality the new class is going to provide. For our example we will have it as an interface which has a function that writes a PageXml. If it is hard to come up with a good abstraction, we can postpone this and do it in two steps: first split the classes and then find a good abstraction. Doing this refactor step it results the following code (file 03_PageXmlExport.cs).

// file: http://tinyurl.com/03-PageXmlExport-cs
public interface IPageFileWriter
{
  bool WriteFile(PageXml page, string filePrefix);
}

public class PageXmlExport
{
  private readonly IPageFileWriter fileWriter;

  private readonly int maxSalesOrders; // used in 3/5 methods
  private readonly bool addCustomerDetails; // used in 2/5 methods

  private readonly ICrmService crmService; // used in 3/5 methods
  private readonly ILocationService locationService; // used in 3/5 methods

  public PageXmlExport(  IPageFileWriter fileWriter,
                         int maxSalesOrders,
                         bool addCustomerDetails,
                         ICrmService crmService,
                         ILocationService locationService)
  { ... }

  public bool ExportCustomerPage(string customerName)
  {
     PageXml content = new PageXml {Customer = new CustomerXml
                                   {Name = customerName}};

     using (EfRepository repository = new EfRepository())
     {
        if (maxSalesOrders > 0)
        {
          var orders = repository.GetEntities<Order>()
            .Where(o => o.Customer.CompanyName == content.Customer.Name)
            .OrderBy(o => o.OrderDate)
            .Take(maxSalesOrders);

        //enrich content with orders
        // ...
      }

      if (addCustomerDetails)
      {
        var customer = repository.GetEntities<Customer>()
            .Where(c => c.CompanyName == customerName);

        // enrich content with customer data
        // ...
      }
   }

   return fileWriter.WriteFile(content, "CustomerPage");
  }
...
}

This refactor step has reduced the number of fields by groping three of them into one. As a consequence it also reduced the dependencies of the class and it made all its methods better by taking out the code that deals with the details of writing a file.

Now, is the moment to think about a better abstraction for the interface we have created: the IPageFileWriter. We could make it more abstract if we rename it and take out or at least rename the filePrefix parameter of its method. The interface in its form limits its implementation to writing to files. A better one would be:

public interface IPageWriter
{
  bool Write(PageXml page);
}

This interface may have implementations that could write the XML anywhere. By increasing this abstraction we are increasing the reusability of the PageXmlExport class, by being able to reuse the same code and logic to export in multiple mediums. We are not going to pursue this refactoring path, but we are going to continue with increasing the cohesion of the PageXmlExport.

Lets see other groups of fields that could be extracted. Looking at maxSalesOrders or addCustomerDetails it is hard to see with which to group them. On the other hand we observe that crmService and locationService are used by the functions that enrich the XML with external data. Therefore one idea is to group these two together in a new depenedency. Because it gives data for the export class we name it as IExportDataProvider interface. It results the following code (file 04_PageXmlExport.cs)

// file: http://tinyurl.com/04-PageXmlExport-cs
public interface IExportDataProvider
{
  CustomerInfo GetCustomerInfo(string name);
  Coordinates GetCoordinates(string city, string street, string number);
}

public class PageXmlExport
{
  private readonly IPageFileWriter fileWriter;
  private readonly IExportDataProvider dataProvider;

  private readonly int maxSalesOrders; // used in 4/5 methods
  private readonly bool addCustomerDetails; // used in 2/5 methods

  public PageXmlExport(  IPageFileWriter fileWriter,
                         int maxSalesOrders,
                         bool addCustomerDetails,
                         IExportDataProvider dataProvider)
 { ... }
...
}

The number of fields and dependences got smaller after this step. However, the code of the class did not improve much. It is the same, but instead calling functions from ICrmService respectively ILocationService, now all the calls go to the same IExportDataProviderIExportDataProvider implementations will wrap together the external services for the export class, but something doesn’t smell good about it. The first bad signal comes from the name. It is named data provider, but it also gets geo location coordinates. Looking  at it from another perspective, we can also see that the interface mixes different levels of abstraction. The CustomerInfo is a high level concept in comparison with Coordinates.

Looking again at the resulted code in PageXmlExport class, we see that it now has this IExportDataProvider, but it also uses a repository to get data from the database. Maybe a better idea would have been to wrap also the repository in the IExportDataProvider. The repository is another dependency our class has, but it is not that visible. The refactoring that we have done made us think to the dependencies more, and now with the IExportDataProvider as a visible dependency in the constructor and the repository being used in most of the methods, it is much more obvious that they should be grouped together as one dependency.

Lets rollback the last refactor step that we did, and wrap the ICrmService together with the repository in a real IExportDataProvider. Thinking on doing this we also observe that the other dependencies our class has (maxSalesOrders and addCustomerDetails settings) are only used together with the hidden dependency towards the repository. They all could be wrapped into the IExportDataProvider. The IExportDataProvider implementations should depend on these settings and them should instruct the repository accordingly. In fact, by doing this we are taking out from the PageXmlExport class the responsibility of being setup about how to read data and we push that lower to IExportDataProvider implementations. The resulted code after this refactor step is the following (file 05_PageXmlExport.cs).

// file http://tinyurl.com/05-PageXmlExport-cs
public class PageXmlExport
{
  private readonly IPageFileWriter fileWriter;
  private readonly IExportDataProvider_5 dataProvider;
  private readonly ILocationService locationService;

  public PageXmlExport(     IPageFileWriter fileWriter,
                            IExportDataProvider dataProvider,
                            ILocationService locationService)
  {
    this.fileWriter = fileWriter;
    this.dataProvider = dataProvider;
    this.locationService = locationService;
  }

  public bool ExportCustomerPage(string customerName)
  {
     PageXml content = new PageXml {Customer = new CustomerXml
                                   {Name = customerName}};

     IEnumerable<CustomerData> orders =
                     dataProvider.GetCustomerOrders(customerName);

     // enrich content with orders
     // ..

     // enrich content with customer data
     // ..

     return fileWriter.WriteFile(content, "CustomerPage");
   }

   public bool ExportCustomerPageWithExternalData(
                                           string customerName,
                                           PageData externalData)
   {
      PageXml content = new PageXml {Customer = new CustomerXml
                                    {Name = customerName}};

      if (externalData.CustomerData != null)
      {
          // enrich with externalData.CustomerData
          // ...
      }
      else
      {
          CustomerInfo customerData =
              dataProvider.GetCustomerInfo(content.Customer.Name);
      }

      IEnumerable<CustomerData> orders =
                     dataProvider.GetCustomerOrders(customerName);

      // enrich content with orders
      // ...

      // enrich content by merging the external customer data with what read from DB
      // ...

      foreach (var address in content.Customer.Addresses)
      {
         Coordinates coordinates = locationService.GetCoordinates(address.City, address.Street, address.Number);
         if (coordinates != null)
            address.Coordinates = string.Format("{0},{1}", coordinates.Latitude, coordinates.Longitude);
      }

      return fileWriter.WriteFile(content);
   }

   public bool ExportOrders(string customerName)
   {
      PageXml content = new PageXml {Customer = new CustomerXml
                                    {Name = customerName}};

      IEnumerable<CustomerData> orders =
                     dataProvider.GetCustomerOrders(customerName);

      //enrich content with orders

      return fileWriter.WriteFile(content);
   }
...
}

Looking now at the result, we are definitely in a much better place than when we were when we started. Maybe our code design is not perfect nor the best, but it is definitely better. Now our PageXmlExport class has three dependencies, which are used by most of its functions. Most of the code in the class got smaller and simpler. The methods follow a simple pattern: they get data using the IExportDataProvider, they enrich the content XML with it (which is the core functionality and knowledge of our class), and they output the result. The details of reading data and output the result are not important. Our class can focus on XML enrichment only.

Maybe we do not yet have the best abstractions in place, but now after we have separated better the concerns it is a good moment to think more about abstraction and encapsulation. The code that gets data from the database or other sources is now decoupled from the core code that enriches the XML. We have achieved the same decoupling for the code that writes the XML on the disk. The details of how these are done are no longer mixed with XML enrichment logic and by abstracting them we can now reuse the XML enrichment code to read data from any medium and output the results to any medium. In conclusion we have achieved a better Separation of Concerns, and a more loosely coupled design, by following a simple refactoring pattern. We did this almost in a mechanical way looking to reduce the number of parameters or the number of dependencies.

Part 3: summarizing the refactoring pattern

If we recap the refactoring pattern we have followed to achieve loose coupling by improving coherence we have the following steps or recommendations:

  1. Be critical with functions that have more than about 4 parameters. It may be that they are part of poor cohesive classes or in they have a poor cohesion themselves.
    • reduce the number of parameters of functions by transforming the common (redundant) ones into class fields received through the constructor.
    • do this in small refactoring steps
    • this may reveal a class with poor cohesion
  2. Be critical with classes that contain more than about 7±2 fields (more toward the high end of 7±2 if the fields are primitives and more toward the lower end of 7±2 if the fields are pointers to complex objects or services)
    • the number 7±2 has been found to be a number of discrete items a person can remember while performing other tasks
    • reduce the number of fields by grouping the ones that are used by same functions into one new field. This leads to taking out code, and it increases the coherence of the original class
  3. Be critical with classes that have more than about 4±1 dependencies
    • most probably they have poor cohesion
    • most probably the dependencies may be reduced by grouping and extracting them in new classes. This increases the cohesion and leads to a loosely coupled code
    • think about putting in place good abstractions for new extracted classes. This will increase the reusability and extensibility of the code.
    • think about different levels of abstractions and different levels reasons of change when you try to improve the abstractions that define the interaction between your classes

By following these with the above example we have refactored from a class that mixes code with details of creating and writing into files, with code that implements high level business logic (XML enrichment) and with code that deals with data access concerns, into a more decoupled design where all these concerns are separated and may be abstracted better.


many examples like the above are included in my Code Design Training

Posted in .NET, Design, Technical, Training | Tagged , , , , , , | Leave a comment

Who Disposes Your Repository

Recently, I’ve went again through the discussion of how the Repository Pattern works with Dependency Injection (DI) in one of the projects I’m involved in. Even if these patterns are around for a while and there are many examples on how to use them together, discussing some particular implementation aspects is still interesting. So, I think it might help to go once again through it, maybe from a different angle.

The case I want to focus on, is when the repository implementation is abstracted through a generic interface, and its implementation is injected through DI to the classes that need to read or store data.

Using DI in an application it is often a good idea. It is an easy way to follow the Separate Construction and Configuration from Use principle (Uncle Bob explains it in his Clean Code book and also Martin Fowler does it here). Abstracting the repository through an interface is also good. It doesn’t only benefit you in writing isolated Unit Tests, but it brings a good separation of the data access concerns from the business logic. It prevents the data access specifics to leak into the upper layers, if you encapsulate them well into the implementation of the interface. Using them together is also good, but you need to consider the stateful nature of a repository and the deferred execution of an IQueryable<T>.

The question I want to address is who disposes my repository implementation and when. The repository will use some connection to the storage, which should be released when is no longer needed. And that should be in a deterministic way, not when the garbage collector kicks in. If the implementation of my repository is through the Entity Framework (EF), it means that I should dispose the DbContext when I no longer need it. For the EF case this may not be too problematic since it does a clever connection management. Here is a response of the EF team on this. However, it may not be the same for other ORMs. Even more I think that IDisposable pattern should be correctly implemented and followed in all cases. I read IDisposable as a clear statement that says that the instances of its implementation must be cleaned in a deterministic way.  I don’t think it’s a good idea to just leave instances undisposed, because I may risk nasty leaks.

I prefer to have an open interface for my repository by taking the benefits the IQueryable<T> brings. This means that the user of my repository can write or alter the query by itself, and I will not need to add a new methods to my repository each time a new use case needs a different kind of filtering or a different data projection. This is easily achieved with an interface like this:

interface IRepository
{
     IQueryable<T> GetEntities<T>();
     ...
}

The clients of my repository may add additional filtering or data selection before the query is actually executed on the database server. Combining it with DI we may have class services like this using it:

class SalesOrdersService : ISalesOrdersService
{
 // The concern of calling repository.Dispose()
 // is not in this service
private readonly IRepository repository;
...

public SalesOrdersService(IRepository repository, ...)
{
 this.repository = repository;
 ...
}

public decimal GetHighRiskOrdersAmount(int year)
{
   IQueryable<Order> orders = GetHighValueOrders()
                            .Where(o => o.Year == year);

   decimal amount = 0;
   foreach (var order in orders)
   {
      if (IsHighRisk(order))
         amount += order.OrderLines.Sum(ol => ol.Amount);
    // only read DB the order lines of high risk orders.
    // important if expect that only 10% of orders are high risk
   }

   return amount;
}

public int CountHighRiskOrders(int startWithYear, int endWithYear)
{
   IQueryable<Order> orders = GetHighValueOrders()
                                   .Where(o =>
                                      o.Year >= startWithYear &&
                                      o.Year <= endWithYear);

// lets say I'm keen on performance and I only want to iterate
//   once through the resultset.
// If I would use the return order.ToArray().Count(IsHighRisk),
//   instead of the foreach,
//   there is one iteration for ToArray and one for
//   the Count()
   int count = 0;
   foreach (var o in orders)
   {
      if (IsHighRisk(o))
         count++;
   }

   return count;
}

public IEnumerable<Order> GetOrders(int startingWith, int endingWith)
{
      return GetHighValueOrders()
                   .Where(order => order.Year >= startingWith &&
                          order.Year <= endingWith);

 // I have the flexibility to:
 // -return IEnumerable, so the query executes when iterated first
 // -return IQueryable, so the query can be enriched by the client before execution
 // -return IEnumerable, but with an List underneath so the query executes here (call .ToList())
}

// This functions makes reuseable the is HighValue evaluation.
// The evaluation of the condition can be translated through
//  LINQ to SQL into a WHERE filtering which runs in the database
private IQueryable<Order> GetHighValueOrders()
{

    var orders = from o in repository.GetEntities<Order>()
                 join ol in repository.GetEntities<OrderLine>() on o.Id equals ol.OrderId
                 where ol.Amount > 100
                 select o;

    return orders;
}
...
}

With this approach, I also get the benefit that for a new use case, I could maybe only wire-up in a different way some of my existent services, reusing them, and all will get through DI the same instance of the repository. Therefore, the underlying implementation has the opportunity to send all the queries from one session / request on the same storage connection. Even more, if my services return the IQueryable<T> as IEnumerable, the query will be executed only when the client needs it.

Coming back to disposing the underling DbContext, one option is to make the repository implementation to also implement IDisposable, and to call DbContext.Dispose() when it is disposed:

class Repository : IRepository, IDisposable
{
   private MyDbContext context = new MyDbContext();

   public IQueryable<T> GetEntities<T>()
   {
      return context.Set<T>().AsNoTracking();
   }

   public void Dispose()
   {
      context.Dispose();
   }
...
}

Now, I have all the benefits I described above, but who is going to call the Dispose() of my repository implementation. The services should not do it. They do not know that the implementation they got injected is an IDisposable and they shouldn’t know it. Making them also to implement IDisposable, and to check on Dispose() if some of their members are IDisposable, is not a solution either. My rule of thumb is that the code that creates an IDisposable instance, should also call dispose on it. It is also according with the Code Analyses Rules. In our case this is the DI Container. Our code does not create, nor explicitly asks for the instance, it is just injected into it. By using DI I am inverting the control of creating instances from my own code to the framework. The framework should also clean them up when no longer needed.

For this, we need to make sure that two things happen:

  1. the DIC will call Dispose() on all the disposable instances it created, and
  2. it will do it in a deterministic way (when no longer needed, meaning when the request  / session ends)

If you’re using Unity Container you have to take care of both. When an instance of the UnityContainer is disposed, it will call Dispose() on all the lifetime managers which are IDisposable. However, the built-in lifetime managers Unity provides for short lived instances, are not disposable, so you need to write your own. Here are some examples on how to do it. Other DIC, like MEF, have this built in.  The other thing to care of is: when the Dispose() call chain will be kicked off. For this you need to use Scoped Containers (aka Container Hierarchies). In short, this means that you will need to associate a request / session with a child container instance, and dispose that child container when the request ends. The dispose of the child container will trigger the dispose of all IDisposable instances it created. Here is a simple example on how to do this for an ASP.NET MVC application, where a child container is associated with each HTTP Request.

Even if this approach gives a lot of flexibility and advantages, is not easy to setup. It requires some non-trivial code to be written. In some applications the added complexity may not be justified. Let’s explore other ideas, too.

We could make the IRepository interface to be IDisposable and not use DI to get the repository implementation, but use Service Locator instead. The main difference from the above is that we are no longer inverting the control. Our code is now in charge of explicitly ask for a repository, so it should also take care of cleaning it up when it is no longer needed. Now, we don’t need to go through all the trouble of making the DI to call Dispose(), because our code will do it. The services now can use a using statement for each repository instance, like:

class SalesOrdersService : ISalesOrdersService
{
 private readonly IServiceLocator sl;

 public SalesOrdersService(IServiceLocator serviceLocator)
 {
    this.sl = serviceLocator;
 }

 public decimal GetHighRiskOrdersAmount(int year)
 {
    using (IRepository repository = sl.GetInstance<IRepository>())
    {
      IQueryable<Order> orders = GetHighValueOrders(repository)
                                      .Where(o => o.Year == year);

      decimal amount = 0;
      foreach (var order in orders)
      {
         if (IsHighRisk(order))
            ammount += order.OrderLines.Sum(ol => ol.Ammount);
      }

      return ammount;
    }
  }

 public IEnumerable<Order> GetOrders(int startingWith, int endingWith)
 {
    using (IRepository rep = sl.GetInstance<IRepository>())
    {
       return GetHighValueOrders(rep)
                     .Where(order => order.Year >= startingWith &
                                     order.Year <= endingWith);
    }
    // Is the above code correct? Have another look

    // the returned IEnumerable will be enumerated by the caller
    //    code after the rep.Dispose() --> error. 

    // To avoid this more (uncontrollable) approaches may be taken
    //     --> error prone --> low maintainability

    // -receive the repository instance created by the caller code.
    //     The caller code creates it, so it needs to dispose it, not me

    // -call .ToList() on this, but still return it as IEnumerable
    //    What happens, when the caller iterates the
    //      order.OrderLines? --> error

    // -this function is refactored to only send a query,
    //    which should be wrapped and executed by the caller
 }

We are losing some of the flexibility and advantages of the above. The main one is that now an IQueryable<T> issued by the repository is scoped to the using statement, not to the request / session. If it is passed as return value to another class, and it gets executed after the using statement ends, an error will occur because the underlying connection is closed. Therefore, I need to be careful that the return values of the services that use a repository should be executed. This means that my clients cannot add to the query additional filters, nor they can decide when / if to execute it. This reduces the flexibility and reusability of my code. This may not be that bad in some applications. One of the biggest risks I see with this is that by trying to workaround these limitations, there may be code that passes around an undisposed repository to different classes or functions (like GetHighValuesOrders() function has). Without discipline, this may get out of hand.

Another different approach is not to make the repository implementation disposable, and to dispose the underlying connection (DbContext), immediately after it is no longer needed. This implies that the IQueryable<T> (better said LINQ to SQL), will not leave the repository scope. This makes the repositories to be use case specific, with a close interface.

class OrdersRepository : IOrdersRepository
{
   public IEnumerable<Order> GetOrders(int startingWith,
                                       int endingWith,
                                       decimal highValueAmount)
   {
      using (var db = new MyDbContext())
      {
          var orders = from o in db.Orders
                  join ol in db.GetEntities<OrderLine>() on o.Id equals ol.OrderId
               where o.Year >= startingWith &&
                     o.Year <= endingWith &&
                     ol.Amount > highValueAmount
               select o;

          return orders.ToList();
      }
   }
   ...
 }

This approach is the most rigid and it makes me add new functions and new repositories each time a functionality is added or changed. I usually do not use it, unless I am dealing with a storage which doesn’t support LINQ.

I prefer to implement the first approach described above, even more in large applications, with many use cases. It is a better choice when you are dealing with more types of implementations which use expensive external resources, which need to be released as soon as possible in deterministic way. In other words, I am addressing with this all the IDisposable implementations (not only the repository), which may be injected through DI.

The first approach and the second one are not exclusive. I may get through DI the repository for some cases and use the service locator where I explicitly want to be more closed to my clients and I would need to dispose the repository even sooner than the request / session would end. I very often use them together: the repository through DI for read only cases, where I would want to more flexibility (more reads on same repository instance), and the repository through Service Locator for read write cases, where I want a smaller scope for my unit of work.


many design discussions like the above are included in my Code Design Training

Posted in .NET, Design, Technical | Tagged , , , , , , , , , , | Leave a comment