Sunday, 30 November 2014

New Chapter: Protocols

Hi!

New chapter on protocols was added. This chapter deals with the idea of creating a stable communication patterns between object and its collaborators. Understanding this idea is crucial for working effectively with mock objects.

Happy reading!

Saturday, 29 November 2014

Three new chapters!

Hi!

Just to let you know - three new chapters were added since the last time I blogged:

Happy reading and I hope this makes up for long absence of posts here!

Sunday, 10 August 2014

Why do we need composability?

Hi,

I moved my book to leanpub (as you may have guessed from the sidebar of this blog). The online version looks very good and is available for all, so I decided there is no point reproducing it on the blog.

Instead, you can read the new chapter called Why do we need composability? online on leanpub and maybe decide to download the current state of the book (which is an always will be free).

This chapter is a next step in explaining the object oriented design knowledge necessary to understand mock objects well.

Have fun!

Friday, 16 May 2014

Road To Mock Objects, pt. 2: Telling, not asking

(this post is adapted from my work-in-progress open source TDD tutorial)

In this post, we'll get back to Johnny and Benjamin as they introduce another change in the code they are working on. In the process, they discover the impact that return values and getters have on composability of objects.

Contractors

Johnny: G'morning. Ready for another task?

Benjamin: Of course! What's next?

Johnny: Remember the code we worked on yesterday? It contains a policy for regular employees of the company. But the company wants to start hiring contractors as well and needs to include a policy for them in the application.

Benjamin: So this is what we will be doing today?

Johnny: That's right. The policy is going to be different for contractors. While, just as regular employees, they will be receiving raises and bonuses, the rules will be different. I made a small table to allow comparing what we have for regular employees and what we want to add for contractors:

Raise Bonus
Regular Employee +10% of current salary if not reached maximum on a given pay grade +200% of current salary one time after five years
Contractor +5% of average salary calculated for last 3 years of service (or all previous years of service if they have worked for less than 3 years) +10% of current salary when a contractor receives score more than 100 for the previous year

So while the workflow is going to be the same for both a regular employee and a contractor:

  1. Load from repository
  2. Evaluate raise
  3. Evaluate bonus
  4. Save

the implementation of some of the steps will be different for each type of employee.

Benjamin: Correct me if I am wrong, but these "load" and "save" steps do not look like they belong with the remaining two - they describe something technical, while the other steps describe something strictly related to how the company operates...

Johnny: Good catch, however, this is something we'll deal with later. Remember the scout rule - just don't make it worse. Still, we're going to fix some of the design flaws today.

Benjamin: Aww... I'd just fix all of it right away.

Johnny: Ha ha, patience, Luke. For now, let's look at the code we have now before we plan further steps.

Benjamin: Let me just open my IDE... OK, here it is:

public class CompanyPolicies 
{
  readonly Repository _repository; 

  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
  
  public void ApplyYearlyIncentivePlan()
  {
    var employees = _repository.CurrentEmployees();

    foreach(var employee in employees)
    {
      var payGrade = employee.GetPayGrade();

      //evaluate raise
      if(employee.GetSalary() < payGrade.Maximum)
      {
        var newSalary 
          = employee.GetSalary() 
          + employee.GetSalary() 
          * 0.1;
        employee.SetSalary(newSalary);
      }
      
      //evaluate one-time bonus
      if(employee.GetYearsOfService() == 5)
      {
        var oneTimeBonus = employee.GetSalary() * 2;
        employee.SetBonusForYear(2014, oneTimeBonus);
      }
      
      employee.Save();
    }
  }
}

Benjamin: Look, Johnny, the class in fact contains all the four steps you mentioned, but they are not named explicitly - instead, their internal implementation for regular employees is just inserted in here. How are we supposed to add the variation of the employee type?

Johnny: Time to consider our options. We have few of them. Well?

Benjamin: For now, I can see two. The first one would be to create another class similar to CompanyPolicies, called something like CompanyPoliciesForContractors and implement the new logic there. This would let us leave the original class as is, but we would have to change the places that use CompanyPolicies to use both classes and choose which one to use somehow. Also, we would have to add a separate method to repository for retrieving the contractors.

Johnny: In addition, we would miss our chance to communicate through the code that the sequence of steps is intentionally similar in both cases. Others who read this code in the future will see that the implementation for regular employees follows the steps: load, evaluate raise, evaluate bonus, save. When they look at the implementation for contractors, they will see the same order of steps, but they will be unable to tell whether the similarity is intentional, or is it there by pure accident.

Benjamin: So our second option is to put an if statement into the differing steps inside the CompanyPolicies class, to distinguish between regular employees and contractors. The Employee class would have an isContractor() method and depending on what it would return, we would either execute the logic for regular employees or for contractors. Assuming that the current structure of the code looks like this:

foreach(var employee in employees)
{
  //evaluate raise
  ...
      
  //evaluate one-time bonus
  ...
  
  //save employee
}

the new structure would look like this:

foreach(var employee in employees)
{
  if(employee.IsContractor())
  {
    //evaluate raise for contractor
    ...
  }
  else
  {
    //evaluate raise for regular
    ...
  }

  if(employee.IsContractor())
  {
    //evaluate one-time bonus for contractor
    ...
  }
  else
  {
    //evaluate one-time bonus for regular
    ...
  }
  
  //save employee
  ...
}

this way we would show that the steps are the same, but the implementation is different. Also, this would mostly require us to add code and not move the existing code around.

Johnny: The downside is that we would make the class even uglier than it was when we started. So despite initial easiness, we'll be doing a huge disservice to future maintainers. We have at least one another option. What would that be?

Benjamin: Let's see... we could move all the details concerning the implementation of the steps from CompanyPolicies class into the Employee class itself, leaving only the names and the order of steps in CompanyPolicies:

foreach(var employee in employees)
{
  employee.EvaluateRaise();
  employee.EvaluateOneTimeBonus();
  employee.Save();
}

Then, we could change the Employee into an interface, so that it could be either a RegularEmployee or ContractorEmployee - both classes would have different implementations of the steps, but the CompanyPolicies would not notice, since it would not be coupled to the implementation of the steps anymore - just the names and the order.

Johnny: This solution would have one downside - we would need to significantly change the current code, but you know what? I'm willing to do it, especially that I was told today that the logic is covered by some tests which we can run to see if a regression was introduced.

Benjamin: Cool, what do we start with?

Johnny: The first thing that is between us and our goal are these getters on the Employee class:

GetSalary();
GetGrade();
GetYearsOfService();

They just expose too much information specific to the regular employees. It would be impossible to use different implementations when these are around. These setters don't help much:

SetSalary(newSalary)
SetBonusForYear(year, amount);

While these are not as bad, we'd better give ourselves more flexibility. Thus, let's hide all of this behind more abstract methods that hide what actually happens, but reveal our intention.

First, take a look at this code:

//evaluate raise
if(employee.GetSalary() < payGrade.Maximum)
{
  var newSalary 
    = employee.GetSalary() 
    + employee.GetSalary() 
    * 0.1;
  employee.SetSalary(newSalary);
}

Each time you see a block of code separated from the rest with blank lines and starting with a comment, you see something screaming "I want to be a separate method that contains this code and has a name after the comment!". Let's grant this wish and make it a separate method on the Employee class.

Benjamin: Ok, wait a minute... here:

employee.EvaluateRaise();

Johnny: Great! Now, we've got another example of this species here:

//evaluate one-time bonus
if(employee.GetYearsOfService() == 5)
{
  var oneTimeBonus = employee.GetSalary() * 2;
  employee.SetBonusForYear(2014, oneTimeBonus);
}

Benjamin: This one should be even easier... Ok, take a look:

employee.EvaluateOneTimeBonus();

Johnny: Almost good. I'd only leave out the information that the bonus is one-time from the name.

Benjamin: Why? Don't we want to include what happens in the method name?

Johnny: Actually, no. What we want to include is our intention. The bonus being one-time is something specific to the regular employees and we want to abstract away the details about this or that kind of employee, so that we can plug in different implementations without making the method name lie. The names should reflect that we want to evaluate a bonus, whatever that means for a particular type of employee. Thus, let's make it:

employee.EvaluateBonus();

Benjamin: Ok, I get it. No problem.

Johnny: Now let's take a look at the full code of the EvaluateIncentivePlan method to see whether it is still coupled to details specific to regular employees. Here's the code:

public void ApplyYearlyIncentivePlan()
{
  var employees = _repository.CurrentEmployees();

  foreach(var employee in employees)
  {
    employee.EvaluateRaise();
    employee.EvaluateBonus();
    employee.Save();
  }
}

Benjamin: It seems that there is no coupling to the details about regular employees anymore. Thus, we can safely make the repository return a combination of regulars and contractors without this code noticing anything. Now I think I understand what you were trying to achieve. If we make interactions between objects happen on a more abstract level, then we can put in different implementations with less effort.

Johnny: True. Can you see another thing related to the lack of return values on all of employee's methods in the current implementation?

Benjamin: Actually, no. Does it matter?

Johnny: Well, if Employee methods had return values and this code depended on them, all subclasses of Employee would be forced to supply return values as well and these return values would need to match the expectations of the code that calls these methods, whatever these expectations were. This would make introducing other kinds of employees harder. But now that there are no return values, we can, for example:

  • introduce a TemporaryEmployee that has no raises, by leaving its EvaluateRaise() method empty, and the code that uses employees will not notice.
  • introduce a ProbationEmployee that has no bonus policy, by leaving its EvaluateBonus() method empty, and the code that uses employees will not notice.
  • introduce an InMemoryEmployee that has empty Save() method, and the code that uses employees will not notice.

As you see, by asking the objects less, and telling it more, we get greater flexibility to create alternative implementations and the composability, which we talked about yesterday, increases!

Benjamin: I see... So telling objects what to do instead of asking them for their data makes the interactions between objects more abstract, and so, more stable, increasing composability of interacting objects. This is a valuable lesson - it is the first time I hear this and it seems a pretty powerful concept.

A Quick Retrospective

In this post, Benjamin learned that the composability of objects (not to mention clarity) is reinforced when interactions between them are: abstract, logical and stable. Also, he discovered, with Johnny's help, that it is further strengthened by following a design style where objects are told what to do instead of asked to give away information to somebody who the makes the decision on their behalf. This is because if an API of an abstraction is built around answering to specific questions, the clients of the abstraction tend to ask it a lot of questions an are coupled to both those questions and some aspects of the answers (i.e. what it in the return values). This makes creating another implementation of abstraction harder, because each new implementation of the abstraction needs to not only provide answers to all those questions, but the answers are constrained to what the client expects. When abstraction is merely told what its client wants it to achieve, the clients is decoupled from most of the details of how this happens. This makes introducing new implementations of abstraction easier - it often even lets us define implementations with all methods empty without the client noticing at all.

These are all important conclusions that will lead us towards TDD with mock objects.

Time to leave Johnny and Benjamin for now. In the next posts, I'm going to reiterate on their discoveries and put them in a broader context.

Sunday, 13 April 2014

Road To Mock Objects, pt. 1: On Objects Composability

(this post is adapted from my work-in-progress open source TDD tutorial)

In this post, I will try to outline briefly why objects' composability is a goal worth achieving and how it can be achieved. I am going to start with an example of unmaintainable code and will gradually fix its flaws in the next posts. For now, we are going to fix just one of the flaws, so the code we will end up will not be perfect by any means, still, it will be better by one quality.

In the coming posts, we will learn more valuable lessons resulting from changing this little piece of code and slowly arrive at justification for using mock objects.

Another task for Johnny and Benjamin

Remember Johnny and Benjamin? Looks like they managed their previous task and are up to something else. Let us listen to their conversation as they are working on another project...

Benjamin: So, what's this assignment about?

Johnny: Actually, it's nothing exciting - we'll have to add two features to a legacy application that's not prepared for the changes.

Benjamin: What is the code for?

Johnny: It is a C# class that implements company policies. As the company has just started using this automated system and it was started recently, there is only one policy implemented: yearly incentive plan. Many corporations have what they call incentive plans. These plans are used to promote good behaviors and exceeding expectations by employees of a company.

Benjamin: You mean, the project has just started and is already in a bad shape?

Johnny: Yep. The guys writing it wanted to "keep it simple", whatever that means, and now it looks pretty bad.

Benjamin: I see...

Johnny: By the way, do you like riddles?

Benjamin: Always!

Johnny: So here's one: how do you call a development phase when you ensure high code quality?

Benjamin: ... ... No clue... So what is it called?

Johnny: It's called "now".

Benjamin: Oh!

Johnny: Getting back to the topic, here's the company incentive plan.

Every employee has a pay grade. An employee can be promoted to a higher pay grade, but the mechanics of how that works is something we will not need to deal with.

Normally, every year, everyone gets a raise by 10%. But to encourage behaviors that give an employee a higher pay grade, such employee cannot get raises indefinitely on a given pay grade. Each grade has its associated maximum pay. If this amount of money is reached, an employee does not get a raise anymore until they reach a higher pay grade.

Additionally, every employee on their 5th anniversary of working for the company, gets a special, one-time bonus which is twice their current payment.

Benjamin: Looks like the source code repository just finished synchronizing. Let's take a bite at the code!

Johnny: Sure, here you go:

public class CompanyPolicies : IDisposable
{
  readonly SqlRepository _repository
    = new SqlRepository();
  
  public void ApplyYearlyIncentivePlan()
  {
    var employees = _repository.CurrentEmployees();

    foreach(var employee in employees)
    {
      var payGrade = employee.GetPayGrade();
      //evaluate raise
      if(employee.GetSalary() < payGrade.Maximum)
      {
        var newSalary 
          = employee.GetSalary() 
          + employee.GetSalary() 
          * 0.1;
        employee.SetSalary(newSalary);
      }
      
      //evaluate one-time bonus
      if(employee.GetYearseOfService() == 5)
      {
        var oneTimeBonus = employee.GetSalary() * 2;
        employee.SetBonusForYear(2014, oneTimeBonus);
      }
      
      employee.Save();
    }
  }
  
  public void Dispose()
  {
    _repository.Dispose();
  }
}

Benjamin: Wow, there is a lot of literal constants all around and functional decomposition is barely done!

Johnny: Yeah. We won't be fixing all of that today. Still, we will follow the scout rule and "leave the campground cleaner than we found it".

Benjamin: What's our assignment?

Johnny: First of all, we need to provide our users a choice between an SQL database and a NoSQL one. To achieve our goal, we need to be somehow able to make the CompanyPolicies class database type-agnostic. For now, as you can see, the implementation is coupled to the specific SqlRepository, because it creates a specific instance itself:

public class CompanyPolicies : IDisposable
{
  readonly SqlRepository _repository
    = new SqlRepository();

Now, we need to evaluate the options we have to pick the best one. What options do you see, Benjamin?

Benjamin: Well, we could certainly extract an interface from SqlRepository and introduce an if statement to the constructor like this:

public class CompanyPolicies : IDisposable
{
  readonly Repository _repository;

  public CompanyPolicies()
  {
    if(...)
    {
      _repository = new SqlRepository();
    }
    else
    {
      _repository = new NoSqlRepository();
    }
  }

Johnny: True, but this option has few deficiencies. First of all, remember we're trying to follow the scout rule and by using this option we introduce more complexity to the CommonPolicies class. Also, let's say tommorow someone writes another class for, say, reporting and this class will also need to access the repository - they will need to make the same decision on repositories in their code as we do in ours. This effectively means duplicating code. Thus, I'd rather evaluate further options and check if we can come up with something better. What's our next option?

Benjamin: Another option would be to change the SqlRepository itself to be just a wrapper around the actual database access, like this:

public class SqlRepository : IDisposable
{
  readonly Repository _repository;

  public SqlRepository()
  {
    if(...)
    {
      _repository = new RealSqlRepository();
    }
    else
    {
      _repository = new RealNoSqlRepository();
    }
  }

  IList<Employee> CurrentEmployees()
  {
    return _repository.CurrentEmployees();
  }

Johnny: Sure, this is an approach that could work and would be worth considering for very serious legacy code, as it does not force us to change the CompanyPolicies class at all. However, there are some issues with it. First of all, the SqlRepository name would be misleading. Second, look at the CurrentEmployees() method - all it does is delegating a call to the implementation chosen in the constructor. With every new method required of the repository, we'll need to add new delegating methods. In reality, it isn't such a big deal, but maybe we can do better than that?

Benjamin: Let me think, let me think... I evaluated the option where CompanyPolicies class makes the choice between repositories. I also evaluated the option where we hack the SqlRepository to makes this choice. The last option I can think of is leaving this choice to another, "3rd party" code, that would choose the repository to use and pass it to the CompanyPolicies via constructor, like this:

public class CompanyPolicies : IDisposable
{
  private readonly Repository _repository;

  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
 

This way, the CompanyPolicies won't know what exactly is passed to it via constructor and we can pass whatever we like - either an SQL repository or a NoSQL one!

Johnny: Great! This is the option we're looking for! For now, just believe me that this approach will lead us to many good things - you'll see why later.

Benjamin: OK, so let me just pull the SqlRepository instance outside the CompanyPolicies class and make it an implementation of Repository interface, then create a constructor and pass the real instance through it...

Johnny: Sure, I'll go get some coffee.

... 10 minutes later

Benjamin: Ha ha! Look at this! I am SUPREME!

public class CompanyPolicies : IDisposable
{
  //_repository is now an interface
  readonly Repository _repository; 

  // repository is passed from outside.
  // We don't know what exact implementation it is.
  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
  
  public void ApplyYearlyIncentivePlan()
  {
    //... body of the method. Unchanged.
  }
  
  public void Dispose()
  {
    _repository.Dispose();
  }
}

Johnny: Hey, hey, hold your horses! There is one thing wrong with this code.

Benjamin: Huh? I thought this is what we were aiming at.

Johnny: Yes, with the exception of the Dispose() method. Look closely at the CompanyPolicies class. it is changed so that it is not responsible for creating a repository for itself, but it still disposes of it. This is wrong because CompanyPolicies instance does not have any right to assume it is the only object that is using the repository. If so, then it cannot determine the moment when the repository becomes unnecessary and can be safely disposed of.

Benjamin: Ok, I get the theory, but why is this bad in practice? Can you give me an example?

Johnny: Sure, let me sketch a quick example. As soon as you have two instances of CompanyPolicies class, both sharing the same instance of Repository, you're cooked. This is because one instance of CompanyPolicies may dispose of the repository while the other one may still want to use it.

Benjamin: So who is going to dispose of the repository?

Johnny: The same part of the code that creates it, for example the Main method. Let me show you an example of how this may look like:

public static void Main(string[] args)
{
  using(var repo = new SqlRepository())
  {
    var policies = new CompanyPolicies(repo);

    //use policies for anything you like
  }
}

This way the repository is created at the start of the program and disposed of at the end. Thanks to this, the CompanyPolicies has no disposable fields and it itself does not have to be disposable - we can just delete the Dispose() method:

//not implementing IDisposable anymore:
public class CompanyPolicies 
{
  //_repository is now an interface
  readonly Repository _repository; 

  //New constructor
  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
  
  public void ApplyYearlyIncentivePlan()
  {
    //... body of the method. No changes
  }

  //no Dispose() method anymore
}

Benjamin: Cool. So, what now? Seems we have the CompanyPolicies class depending on repository abstraction instead of an actual implementation, like SQL repository. My guess is that we will be able to make another class implementing the interface for NoSQL data access and just pass it through the constructor instead of the original one.

Johnny: Yes. For example, look at CompanyPolicies component. We can compose it with a repository like this:

var policies 
  = new CompanyPolicies(new SqlRepository());

or like this:

var policies 
  = new CompanyPolicies(new NoSqlRepository());

without changing the code of CompanyPolicies. This means that CompanyPolicies does not need to know what Repository exactly it is composed with, as long as this Repository follows the required interface and meets expectations of CompanyPolicies (e.g. does not throw exceptions when it is not supposed to do so). An implementation of Repository may be itself a very complex and composed of another set of classes, for example something like this:

new SqlRepository(
  new ConnectionString("..."), 
  new AccessPrivileges(
    new Role("Admin"), 
    new Role("Auditor")),
  new InMemoryCache());

but the CompanyPolicies neither knows or cares about this, as long as it can use our new Repository implementation just like other repositories.

Benjamin: I see... So, getting back to our task, shall we proceed with making a NoSQL implementation of the Repository interface?

Johnny: First, show me the interface that you extracted while I was looking for the coffee.

Benjamin: Here:

public interface Repository
{
  IList<Employee> CurrentEmployees();
}

Johnny: Ok, so what we need is to create just another implementation and pass it through the constructor depending on what data source is chosen and we're finished with this part of the task.

Benjamin: You mean there's more?

Johnny: Yeah, but that's something for tomorrow. I'm exhausted today.

Quick Retrospective

In this post, Benjamin learned to appreciate the composability of objects, i.e. the ability to compose object with other objects into more complex structures with more complex behaviors without changing the implementation of the object's class. This means that potentially, a composable object might trigger different behaviors depending on what it is composed with. We'll talk more about this property in the coming posts.

As I said, the code Johnny and Benjamin worked on in this posts still has some serious flaws. For now, our characters did not encounter a desperate need to address those flaws yet, but this is going to change in the next posts.

Also, after we part again with Johnny and Benjamin, we are going to reiterate the ideas they stumble upon in a more disciplined manner.

Sunday, 5 January 2014

Mocks "break encapsulation"? Here's why I disagree...

note that when I say "statement" in this blog post, I mean "unit test".

For some time, I am witnessing discussions whether using mocks is something that "breaks encapsulation" and thus should be used sparingly. The time has come for me to voice my opinion.

Is "breaking encapsulation" even the right charge?

First of all, we must decrypt what "breaking encapsulation" means. It's not so simple actually, for two reasons.

The first reason is that encapsulation is understood differently by different people and is sometimes used synonymously with information hiding and sometimes a distinction is made between the two.

If we decide to simplify things and treat encapsulation as "information hiding", it is still unclear what it means to "break" it. Does it mean to "reveal information"? In this case, almost any object breaks encapsulation, because any object reveals some information. In other words, there's not such thing as "full encapsulation", because if everything was hidden, objects would not be able to communicate at all.

So, what does it mean to "break encapsulation" in case of charges made against mocks?

What is the actual charge?

The thing many people have problem with is that unit-level specification makes statements on how an object uses (or more precisely, communicates with) its collaborators. For example, let's assume that I have a class that sends a packet through a connection and write the following unit-level statement:

[Test] public void
ShouldOpenConnectionThenSendAndThenCloseItWhenAskedToSendPacket()
{
  //GIVEN
  var connection = Substitute.For<Connection>();
  var sending = new Sending(connection);
  var packet = Any.Instance<Packet>();

  //WHEN
  sending.PerformFor(packet);

  //THEN
  Received.InOrder(() =>
  {
    connection.Open();
    connection.Send(packet);
    connection.Close();
  });
}

Note that this statement is coupled not only to the public methods of a class, but also to the way it uses objects of another class. The usual charge made against such statements is based on the following deduction chain:

  1. How an object uses its collaborators is not part of the object's public interface, hence it's an implementation detail
  2. Mocks are all about how an object uses its collaborators
  3. So it seems that statements using mocks are coupled to the implementation details of specified class
  4. If we change the class so that it uses different collaborators or uses them differently, it may still do its job from the perspective of the clients of those objects (who just call the methods and expect return values). But, the statements will be failing in such case, because they're coupled to how object uses its collaborators.
  5. The statements using mocks are brittle
  6. Mocks should be avoided
  7. Well... unless they're necessary to get high code coverage in places where just invoking methods and checking results is not enough

In the next section, I'll try to point why I think this kind of thinking is flawed.

Dependency Injection changes the game

The first point of this deduction chain we just talked about tells that it's object's private business who and how it calls to achieve this goal.

This may even be true for an object that is instantiated like this:

var sending = new Sending();

But is utterly false when the object is instantiates like this:

Connection c = ...; //get object that implements Connection interface
var sending = new Sending(connection);

What is changed, you may ask - it's just adding a constructor parameter! Well, no it's not just this. The point is, the Sending class now can have its dependency injected. This is very important, because the whole point of dependency injection is that the object is not the owner of its dependencies. Thus, such object effectively says: "Listen up, my client, it's your problem what you give me, as long as I am able to use it". In other words, each client of the Sending class can have their own class implementing the Connection interface and each new user of the class can write their own implementation if they wish so.

So now the real question kicks in:

How can anyone provide a custom implementation to work with a class without knowing how the custom implementation will be used?

Remember, you can't just implement all methods of an Connection interface anyway you want. By doing so, you have a nice chance to violate the Liskov Substitution Principle. Thus, the custom implementation of Connection a client provides must not only implement the interface, it must also adhere to a certain protocol required by the Sending class that will be using it.

This leads us to one conclusion: if a custom implementation of Connection must be coded with a knowledge of the protocol between Sending and Connection, this protocol is not a private business of Sending class. While it may not be part of the public API, it's still part of the public contract.

Other examples

There are many examples out there that what I just wrote is true. For example, let's look at JEE Servlet lifecycle documentation and see what we can read there:

The lifecycle of a servlet is controlled by the container in which the servlet has been deployed. When a request is mapped to a servlet, the container performs the following steps.

  1. If an instance of the servlet does not exist, the web container
  2. Loads the servlet class.
  3. Creates an instance of the servlet class.
  4. Initializes the servlet instance by calling the init method. Initialization is covered in Creating and Initializing a Servlet.
  5. Invokes the service method, passing request and response objects. Service methods are discussed in Writing Service Methods.
  6. If it needs to remove the servlet, the container finalizes the servlet by calling the servlet’s destroy method. For more information, see Finalizing a Servlet.

For comparison, let's take a look at what we can read at ASP.NET web page life cycle documentation. Again, there's a table where we can find the names of the methods called at each stage, the order of the methods called and what we can expect when implementing each method.

These aren't necessarily examples of dependency injection, but are examples of inversion of control, which DI is a particular application of.

So, ekhem... would you really say that putting knowledge on what methods and in what order are performed breaks encapsulation of JSP container or ASP.NET runtime? Or, in better words, is it exposing implementation details? No, it's just another part of the class public contract. Another example might be unit-testing frameworks, where you write so called "test classes" knowing, that "Setup" method is called before a test and "teardown" is called after.

And this is my view on the whole encapsulation dillema - as soon as you make the dependencies of the class injectable (or use any other forms of inversion of control), how this class uses those dependencies becomes a part of public contract of a class, not private implementation detail. This public contract should be specified and thus we write unit level statements unit mock objects.

Of course, nothing, never, prevents you from exposing private implementation details while using any technique, it's just that mocks don't seem different in this regard in any particular way.

What do YOU think?

Sunday, 8 December 2013

Moving to NUnit from MsTest - experience report

Recently, I was a part of migration of some tests from MsTest to NUnit. I was in a situation where some of the tests were written in NUnit and some (legacy ones) in MsTest and it was too costly to maintain both tools, so the money were put on NUnit. The goal was to convert existing tests as fast as possible, leaving refactoring and cleanup of test and production code for later. I didn't know much about MsTest before, and here's what I learned about it and the transition:

Assertions

Much to my astonishment, the number of assertions MsTest supports is rather small, compared to NUnit. You cannot assert something is greater than something else, cannot assert a value is in a certain range etc. This way, a lot of assertions looked like this (remember this was ugly legacy test code :-)):

Assert.True(numberOfMessages <= 1000 
  && numberOfMessages >= 950, "numberOfMessages")

This, at failure, gives you the a message like: "Expected true, got false". Pretty helpful, isn't it (sarcasm)? Sure, you can make the message better, but you have to work hard on it yourself. The NUnit's (better) equivalent is:

Assert.That(numberOfMessages, Is.InRange(950, 1000));

Usually, when writing good unit tests, I don't really need this kind of assertions, but this was legacy code and it tested many kinds of weird situations over a large set of objects put together.

On the other hand, MsTest has a strongly typed assertions in form of Assert.AreEqual<T>() which are missing from NUnit. This, however, is not a big deal, because creating a custom wrapping assertions which will give you this benefit in NUnit is trivial. For example:

public class XAssert
{
  public static void AreEqual<T>(T expected, T actual)
  {
    NUnit.Framework.Assert.AreEqual(expected, actual);
  }
}

Lessons learned:

  1. If you ever use MsTest, do yourself a favor and pick an external assertion library.
  2. If you use NUnit, writing strongly typed wrappers over common assertions may be a good idea.

Deployment Items in .testsettings file

MsTest has this little feature of deployment items that can be configured in solution-wide .testsettings file to be copied to the output directory before a test is run (usually these are some extra files etc.).

But wait! Why on earth would someone be willing to use that kind of feature in unit tests at all?

Unfortunately, this was my case - the deployment items were used to load an XML configuration :-(. The problem with Deployment Items is that some 3rd party runners support it, and some don't (namely: NCrunch and Gallio, which makes running tools like Sonar over such tests pretty difficult). Also, some 3rd party unit testing tools do not run tests from the build output directory, but create their owne custom directories somewhere on the hard drive (e.g. NCrunch does), but the paths coded in Deployment Items are not adjusted and get gopied to the same directories as always, leading to failed runs.

Lessons learned:

  1. If you're using Deployment Items in MsTest for unit tests, you don't even give me a chance to think you're serious.
  2. When Deployment Items are used to copy some additional files to the build output, this mechanism can be replaced with including these config files in test project as links and setting their "Copy Local" property to copy them to the build output directory.
  3. Do not write or load files in unit tests!!!

PrivateType and PrivateObject

I was shocked when I discovered that MsTest includes helpers for accessing private instance/static methods: PrivateType and PrivateObject. Even for legacy code, there are better strategies to apply. Anyway, what shocks me more is that anyone can even think that using these may be a good idea (it is not). Thus, when converting to NUnit, I found such constructs in the code. They lead to brittle tests that are accessing members using strings and reflection. It seems like MsTest allows generating accessors that are strongly typed and use strings and reflection under the hood, but this is still coupling to implementation details, plus I saw PrivateType and PrivateObject being used directly many times.

Lessons Learned:

  1. When writing new tests, forget that PrivateType and PrivateObject exist. Look for the real problem.
  2. PrivateType and PrivateObject are just helpers over a reflection API, so they don't require being executed under MsTest to work properly. During transition to NUnit, you can leave references to MsTest assembly and change all code references to include namespace e.g. from PrivateObject to Microsoft.VisualStudio.TestTools.UnitTesting.PrivateObject. The code will still work.

Cost of Transition

In general, it's a lot simpler to migrate from MsTest to NUnit than the other way round. This is because NUnit requires only that an assembly references nunit.framework.dll and marks certain methods with [Test] attribute. On the other hand, MsTest requires a special type of project (a Test Project) to hold the tests. Also, as I said, MsTest's assertions are for the most part a subset of NUnit's. Anyway, to convert a project from MsTest to NUnit, the following steps are usually sufficient:

  1. Reference nunit.framework.dll from the test project.
  2. Convert Deployment Items into links in project copied to the build output folder.
  3. Search-Replace the following:
    1. [TestClass] -> [TestFixture]
    2. [TestMethod] -> [Test]
    3. [TestInitialize] -> [SetUp]
    4. [TestCleanup] -> [TearDown]
    5. [TestClassInitialize] -> [TestFixtureSetUp]
    6. [TestClassCleanup] -> [TestFixtureTearDown]
    7. using Microsoft.VisualStudio.TestTools.UnitTesting; -> using NUnit.Framework;
  4. Change Assert.IsInstanceOfType(x, typeof(X)) to Assert.IsInstanceOf<X>(x);
  5. Change Assert.AreEqual<X>(x, y) to Assert.AreEqual(x, y)
  6. Fix references to some MsTest-specific classes (like PrivateObject)

Summary

And that's it. These were the lessons learned and tips for dealing with the transition as fast as possible. I advise to do a real cleanup of the test and production code sooner than later, but the tips I outlined here let you defer this cost.