Designing for testability – an valid architecture choice? (Part 3)

This is third and final part of design for testability series of posts. 
First part of the series covered initial separation of concerns between manager in provider classes, so in case you haven’t read that already, jump here and  read it first. 
Second part of the series was focused on decoupling the manager and provider class by separating them with usage of service stub pattern with user provider interface. In case you haven’t seen it, check it out here

Redesign step 3 – IUserManager

In part 2 we applied service stub decoupling between user manager and provider by putting internal provider interface between them. In this step, we would in general do the same thing, just this time putting the interface.

As you can probably guess already, we have again the same problem: UserManager is static class with static NumberOfUsersActiveInLast10Days method so we have to remove them if we want to introduce interface usage. But, the difference is in the fact that we could do whatever we wanted with provider class because it was internal, encapsulated class. This time we have a console code using the static manager method so by removing static attributes we would change that code on a significant way  by forcing console to create an instance to access the instance method which could cause some performance issues etc

Today post would show two approaches to solution of that problem:

Both examples would have  common start:

  • removal of the static attributes from UserManager class and NumberOfUsersActiveInLast10Days method
  • extracting an interface IUserManager and implementing it on UserManager

image

Singleton based refactoring

Singleton is very simple pattern, which is based on the idea that if we replace instance constructors with  static factory method we would get the result of instance class behaving like static class.

    public class UserManager : IUserManager
    {
        private UserManager(){/*prevents instance contruction*/}

        public static readonly IUserManager Instance = new UserManager();

In line 3, default constructor is hidden to prevent creating of another instance of UserManager

In line 5,  static field is been defined and set to an instance of Usermanager(). Because of the fact this is static field, this instantiation would happen only once.

Due to the fact that now NumberOfUsersActiveInLast10Days  is an instance method, the way of how it would be invocated in CompanyManager would have to change too, to reflect the new singleton nature of the class.

using System.Collections.Generic;

namespace DAL
{
    public class CompanyManager
    {
        private static IUserManager _userManager = UserManager.Instance;

        internal static IUserManager Manager
        {
            set { _userManager=value; }
        }


        public static IList<int> GetActiveUsers()
        {
            IList<int> result = new List<int>();
            result.Add(_userManager.NumberOfUsersActiveInLast10Days("A"));
            result.Add(_userManager.NumberOfUsersActiveInLast10Days("B"));
            result.Add(_userManager.NumberOfUsersActiveInLast10Days("C"));
            return result;
        }

The changes done in CompanyManager are very similar to the one done in UserManager in previous post and in short they are based on replacement of the UserManager direct usage (in lines 27) with a field declared in line 10 and instantiated to point to newly created Singleton Instance property of the UserManager.

Line 11 provide just a way already seen of how to make an option for changing the real UserManager with something implementing the IUserManager during the run time to allow easy testability and all thanks to dependency injection – setter type

With this code in place, testing of the CompanyManager would be very easy and looked something like this

        [Test]
        public void GetActiveUsers_TestCaseOfZeroUsers2()
        {
            IUserManager userManager = mockRepository.DynamicMock<IUserManager>();
            Expect.Call(userManager.NumberOfUsersActiveInLast10Days(null))
                .IgnoreArguments()
                .Return(0);
            mockRepository.ReplayAll();

            CompanyManager.Manager = userManager;
            IList<int> results = CompanyManager.GetActiveUsers();
            Assert.IsTrue(results.Count == 0);
        }

As we can see on this test, we now just mock the direct dependencies – UserManager behavior without going into the internal details on how manager is implemented. “UserManager would return this and I don’t care here at all how that would be retrieved “

So we got the decoupled and testable design without exposing the internals of how something is been used.

What is wrong with this code?

Now when we cleaned internal relationships between user manager and user provider and decoupled the external relationship UserManager has, the only thing left is the fact that UserManager.NumberOfUsersActiveInLast10Days method still has mixed crosscutting concerns (validation and caching) with business logic which breaks separation of concerns and decreases testability. A lot of people I know would say that this is not a big deal for them, so for them redesigning for testability could stop here because with singleton pattern implementation there is not much what it can be done further on efficient way.

Registry based refactoring

As I stated earlier, there is another type of solution applicable to this problem and in it we won’t make any changes on the UserManager. UserManager class would just lost its static attributes and become and normal instance class. Desired static functionality in this approach will be achieved with a new repository class which would expose unique instances of manager classes through static properties.

Something like this:

namespace DAL
{
    public static class DALRepository
    {
        public static readonly IUserManager UserManager =
            new UserManager();

        public static readonly CompanyManager CompanyManager =
            new CompanyManager();

    }
}

As it can be seen from lines 5 and 8, the normal instances are created and stored in a static repository class which is then used as registry of assembly functionality, removing the need of doing any kind of “being static” targeted development. CompanyManager is just another instance class in this example implementing dependecy injection/service stub design we already saw in this post:

using System.Collections.Generic;

namespace DAL
{
    public class CompanyManager
    {
        private IUserManager _userManager = new UserManager();

        internal IUserManager UserManager
        {
            set { _userManager=value; }
        }


        public IList<int> GetActiveUsers()
        {
            IList<int> result = new List<int>();
            result.Add(_userManager.NumberOfUsersActiveInLast10Days("A"));
            result.Add(_userManager.NumberOfUsersActiveInLast10Days("B"));
            result.Add(_userManager.NumberOfUsersActiveInLast10Days("C"));
            return result;
        }
    }
}

Notice that in Line 7, field is been assigned pointer to new instance of UserManager and that neither class nor method are not static any more. Registry class takes on itself all the burden of adding static attributes.

Testing CompanyManager would in case of repository pattern based solution look like this

        [Test]
        public void GetActiveUsers_TestCaseOfZeroUsers2()
        {
            IUserManager userManager = mockRepository.DynamicMock<IUserManager>();
            Expect.Call(userManager.NumberOfUsersActiveInLast10Days(null))
                .IgnoreArguments()
                .Return(0);
            mockRepository.ReplayAll();

            DALRepository.CompanyManager.UserManager = userManager;
            IList<int> results = DALRepository.CompanyManager.GetActiveUsers();
            Assert.IsTrue(results.Count == 0);
        }

We would create a mocked object of UserManager which would return zero number of users in lines 4-7

In line 10 , we use the DALRepository class CompanyManager static property to set UserManager which would be used by company manager to the mocked one

In line 11, we use the DALRepository class CompanyManager static property to call the GetActiveUsers() method

An additional benefit of Registry pattern based solution

You’ve probably aware that one of the new cool things Enterprise Library 3.1 can offer is Policy Injection Application block which is a very cool way of removing the cross cutting concern type of code from your business logic. The problem I face usually with PIAB block is that I have to enforce anywhere in the application developers to use PoliciyInjection creation factory method instead of the default constructors objects are offering. Repository pattern used on the way just explained, is IMHO a perfect way of how to encapsulate that policy injection creation code.

We cold rewrite very easy DALRepository class to look like this

using Microsoft.Practices.EnterpriseLibrary.PolicyInjection;

namespace DAL
{
    public static class DALRepository
    {
        public static readonly IUserManager UserManager =
            PolicyInjection.Create<UserManager, IUserManager>();

        public static readonly CompanyManager CompanyManager =
            PolicyInjection.Create<CompanyManager>();
    }
}

The code anywhere would still use the DALRepository,UserManager anywhere in the code without being aware of policy injection occurring in background

I won’t go here too deep into the PIAB implementation (I had a session about it already, so in case you care check it out here ) but we can use it easily to perform final redesign of the initial code and remove the validation and caching from business logic.

I would explain here in short how interface based PIAB approach solving this would look.

We would just need to:

  1. decorate IUserManager with next two attributes (lines 5 and 6):
using Microsoft.Practices.EnterpriseLibrary.PolicyInjection.CallHandlers;

namespace DAL
{
    public interface IUserManager
    {
        [CachingCallHandler]
        [ValidationCallHandler]
        int NumberOfUsersActiveInLast10Days(string userName);
    }
}

2. Define appropriate matching rules and active policies in EntLib configuration file

3. Remove the code implementing validation and cahcing

The result will be that method where we have started UserManager.NumberOfUsersActiveInLast10Days would contain only business logic so the it’s coherence and maintainability would be highest possible

        public int NumberOfUsersActiveInLast10Days(string userName)
        {

            IList<User> userCollection = _userProvider.GetUserCollection(userName);
            int result = 0;
            foreach (User user in userCollection)
            {
                if (user.LastActivity > DateTime.Now.AddDays(-10))
                {
                    result++;
                }
            }
            return result;
        }

What is wrong with this code?

As far I can tell, there is nothing wrong with this code any more.

Conclusion

We redesigned starting example for testability and that give us clear separation of concerns, loosely coupled component interactions,high maintainability etc.

I do agree that with usage of TypeMocks we could test the original method too, but then we would loose all the side benefits designing for testability brings. Maybe the whole fuzz is because of the wrong terminology: “design for testability” should be called “design for maintainability”  from the reasons this couple of blog posts I hope showed 🙂

Source code presented in this examples can be found here

Advertisements

Posted on December 2, 2007, in Uncategorized. Bookmark the permalink. 1 Comment.

  1. Nice post, I love the code I get with registry pattern

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: