Design, Test and Develop like it's heaven on Earth - Part 6

Published on 6/27/2019

Following the plan

Looking at the plan from part 5 gives us a nice path to follow to completion. We’ve already detailed exactly where we need to start (the tests!). So, without further ado...

Red


[TestMethod]
public async Task Publish_IsPublished_MarkedTrue()
{        
    //arrange
    Blog blog = new Blog();

    //act
    await Service.Publish();

    //assert
    Assert.IsTrue(blog.IsPublished);
}

So we followed the first item of our plan, and we have a call to Publish() detailed in the test and we assert for the property value. If we follow through and create the code, what do we put into the Publish() method? Where does Publish() get the blog from that it sets the property of to true? The requirements that we planned from says: “I want to select a previously created blog post and mark it as published”. This implies that we’re not passing in the entire blog like when we create it. The only other place we can get the blog from then is the database. But to do that we either have to call the Read() function, or talk directly to the adaptor. Either way, we will need the title for the blog.


[TestMethod]
public async Task Publish_IsPublished_MarkedTrue()
{        
    //arrange
    string title = "hello_test";
    Blog blog = new Blog();

    //act
    await Service.Publish(title);

    //assert
    Assert.IsTrue(blog.IsPublished);
}

And then we code-complete and then fill in the Publish() method.


public async Task Publish(string title)
{        
    var blog = await _dbAdaptor.Read(title);

    blog.IsPublished = true;
}

Our test doesn’t yet pass, we have to make one modification to it so that we can actually use the blog instance that we arrange here for ourselves. We set up the mock to return it.


[TestMethod]
public async Task Publish_IsPublished_MarkedTrue()
{        
    //arrange
    string title = "hello_test";
    Blog blog = new Blog();
    _dbAdaptorMock.Setup(x => x.Read(title))
        .ReturnsAsync(blog);

    //act
    await Service.Publish(title);

    //assert
    Assert.IsTrue(blog.IsPublished);
}

Green

Our test pass. I don’t see anything to refactor at this point. We move one to the next item on the plan, the publish date. There are two tests we need to write.

Red


[TestMethod]
public async Task Publish_NoDate_IsSet()
{        
    //arrange
    string title = "hello_test";
    DateTime expectedDate = DateTime.Today;
    Blog blog = new Blog();
    _dbAdaptorMock.Setup(x => x.Read(title))
        .ReturnsAsync(blog);

    //act
    await Service.Publish(title);

    //assert
    Assert.AreEqual(expectedDate, blog.PublishDate);
}       

[TestMethod]
public async Task Publish_DateSet_IsNotSet()
{
    //arrange
    string title = "hello_test";
    DateTime expectedDate = DateTime.Today.AddDays(-2);
    Blog blog = new Blog() { PublishDate = expectedDate };
    _dbAdaptorMock.Setup(x => x.Read(title))
        .ReturnsAsync(blog);

    //act
    await Service.Publish(title);

    //assert
    Assert.AreEqual(expectedDate, blog.PublishDate);
}

Only one test doesn’t pass. The date is not set if it’s null. We can update the code of the Publish() method to fix it.


public async Task Publish(string title)
{        
    var blog = await _dbAdaptor.Read(title);

    blog.IsPublished = true;
    blog.PublishDate = blog.PublishDate ?? DateTime.Today;       
}

Green

Our test now passes. Is there anything we can refactor? That line of code we just added, we’ve seen that before. When we were working on the Create() method, we had this line of code as well. At the end of that red/green/blue flow, we had moved that code into a validation method.

Blue

Perhaps we can just call that method here too?


public async Task Publish(string title)
{        
    var blog = await _dbAdaptor.Read(title);

    blog.IsPublished = true;

    Validate(blog);
}

Is that the right thing to do? It makes sense to validate the blog one last, final time before it is marked as published, right? Let’s test it. This is the power of unit testing. Suddenly we have three failed tests. Three! Wow. Clearly, this has a big impact. And they are all the new publish testing methods. So, why are they failing?


System.ArgumentNullException: Value cannot be null.
Parameter name: Title

Well, of course! The first thing that is validated is that we should have a title so that it can be turned into a key for storing it into the database. So how does that play for our Publish() method? Well, it actually loads the blog by the title from the database adaptor, so I think here we can assume that the blog will have a title filled in already. After all, it must have been saved previously with a title. It’s just that we didn’t fill it in when we supplied it to our mock. And if it doesn’t have a title, if something is wrong with the data, it will let us know immediately through failing fast and throwing that ArgumentNullException, just like the unit tests do. It feels like a real win to validate the blog entry again now. Let’s add the title in all these tests, and run it again.


Blog blog = new Blog() { Title = "Hello Test!", PublishDate = expectedDate };

And suddenly all our tests pass!

SOLID: S for Single Responsibility

We created the Validate() method previously, and now we were able to reuse it. Similarly, we created the Read() method before also, and we could also have reused that too. This was only possible because those two methods have one single purpose each, and they do only what it says on the box. If these methods had a dual purpose or if the outcomes were determined by some conditions hidden inside them or if they had a dependency on something, it would have been much harder to reuse them in a different context or with a different dependency. When you write simple and deterministic methods with no or very little dependencies, you will always be able to trust them and reuse them. It will also be really easy to test them with unit tests.

Why not call Read() also?

So I did not call the Read() method of the service and instead read directly from the adaptor. It would not have been wrong to do it, but my decision is based on the following reason: I don't want to mix business rules or create dependencies between different business rules. If the Publish() method now relies on the Read() method, and the definition of the Read() method has to change at some point in the future, my Publish() method will be affected. For example, perhaps we need to add some additional transform into the Read() method to fulfill a change in business rules. Such a change will then have side effects on my other business rule.

Of course, it is also entirely plausible that we might need to make a change in how the adaptor presents the data as part of its own Read() method. Which would affect both the service Read() and Publish() methods. But this will also require a change in the abstraction (perhaps a different parameter, or a change to the Blog model?). So this decision is all about where you want to put your risk. There is no wrong or right answer here - I'm opting to put my risk all in one basket (the adaptor's Read() method) instead of spreading my risk around via dependencies. And typically lessening dependencies is always more beneficial, as we've already seen (and will again).

Conclusion

In this part we worked partially through our plan and built some of the unit tests on the list. We saw how the unit tests helped us execute our actual code early and often, through each step of your development process, to continually reaffirm our understanding of both the problem and the solution. And even though we did not use one of the principles in this part, we saw how it helped us that we applied it before and that we were able to reuse code. You might remember in part 5 we already saw as part of our plan that we’re repeating the unit tests for the publish date that we did in the Create() method work. Our principles and our plan align! This leaves us feeling at ease for the future. And if the requirements around the published date now change, we only have one place to go and make that change. And we have a bunch of unit tests that will guard us when we make those changes.