Skip to content

Index

Mars Rover - Intro To Testing

What This Is and What This Isn’t

The goal of this post isn’t to try to convince you that you should be unit testing your code, but to give you enough information that if you need to unit test your code or if you’re in a codebase that expects tests, you will have the knowledge to hold your own. If you are looking for reasons why you should be testing, there are some great resources in the Additional Resources section at the end of the post!

Unit Testing 101

At its core, unit testing is the practice of writing code that tests that your code is working correctly. Confused? It’ll make sense in a minute, I promise! If you’ve ever made changes to a project, how confident were you that your changes worked? If I had to guess, there’s a high level of confidence that your changes solved the problem. However, how confident were you that your changes didn’t break some other piece of functionality? I imagine that your confidence is a bit lower. For me, I’m reasonably confident that my changes are solid, but I’m not always sure that I didn’t break some other features elsewhere. The most straightforward approach to verify everything is working correctly is to run the application and try it out, right?

For small applications, that’s a pretty reasonable approach to take and wouldn’t take too much time. However, what if you’re working on a more complicated application? How much time is it taking for you to compile the application, launch it, and start navigating through the UI? The premise of unit testing is that we can exercise the same logic that’s running in the application but without having to go through the interface. This type of testing is generally faster and less error-prone since your test code will do the same tests over and over again. The downside is that you’ll spend more time writing code, but for me, I feel much more confident in my work when I can run the full test suite and know pretty quickly if I’ve broken something.

Naming Again?!

If you’ve been following along with the Mars Rover kata series then you know I’m a huge fan of using the same language as my Subject Matter Experts (SMEs) when it comes to the problem at hand as it prevents confusion when different terminology is used for the same concept.

When it comes to naming my tests, I take this approach one step further and name my tests in such a manner that if you read the class name followed by the method, then you’ll have a clear idea of what the test’s intent is. The main reason I name my tests this way is that if my test fails, I want to know what requirement or workflow is not working so I can see if it makes sense for that workflow to be impacted.

Sometimes, a test will start to fail because requirements have changed, so the test needs to be updated or removed. In other cases, the test failure reveals a contradiction in rules so it helps me if I can clearly see the use case and ask the right questions to my SMEs.

Framing Context with Given/When/Then

When it comes to naming my test classes and method, I borrow inspiration from the Given/When/Then naming methodology. I came across this convention when learning about Behavior Driven Development (BDD) early on in my career but I was familiar with the naming convent from working with user stories. The intent of Given/When/Then is that it provides the context, an action, and a result. Going to Mars Rover, an example user story would be

Given that the rover is at (0, 0) facing North, when it receives the move forward command, then it should be at (0, 1) facing North

Let’s break this user story down and examine the various parts:

Given

The Given portion of the user story sets up the context of the application. It can be as simple as Given that the user is logged into the system or as complex as Given that today is a holiday and that a user wants to make a reservation. The goal here is that someone can read this portion of the story and understand what is being accomplished. For this user story, the context is that we have a Rover that’s located at (0, 0) facing North.

When

The When portion of the user story gives what action is being taken in the application. At this level, we’d typically stay away from technical jargon (i.e. the user clicks the reservation button) and focus more on the workflow being accomplished. For this user story, the action is that the Rover received the Move Forward command.

Then

The Then portion of the user story tells us what the expected result or behavior is. It can be as simple as then the total is $42.55 or as complex as then the sale is declined and the user is informed that there was an issue processing the payment. For this user story, we’re expecting that after the Rover receives the Move Forward command, then the rover is at (0, 1) facing North.

Test Structure and Naming Conventions

Now that we’ve learned a bit more about Given/When/Then, let’s talk about how this can influence your test structure. When I’m writing tests, I’ll typically place them in a unit test project that’s separate from production code so that we don’t deploy the unit tests with the production code.

In the case of Mars Rover, if I have a project called MarsRover, then I’d have a project called MarsRover.UnitTests. Once I have this new project created, I’ll create a folder called xyzTests where xyz is the name of the class I’m writing tests against. So if I’m writing tests against the Rover class in the MarsRover project, then I would have a folder called RoverTests in the MarsRover.UnitTests project.

Solution Layout for Mars Rover in VS Code

Project naming conventions outlined in red. The test class and folder naming conventions outlined in blue

From here, I’ll generally create a file per method or workflow that I want to test for the class. So based on our user story above, I would have a file called WhenMovingForward and this file will contain all the different tests for this functionality. Once a file is in place, we can start writing the various test methods for different behaviors. When naming methods, I will include the context for the setup and what the expectations were. By combining the test name and the method name, it will sound like a user story.

Shows the test class with names in the test runner

VS Code Test Runner showing test classes with test names

Organizing Your Test With Arrange/Act/Assert

At this point, we have the infrastructure in place for our tests, so how do we write a good test? Every good test will have three parts, Arrange, Act, and Assert. In the Arrange step, we’re going to focus on creating dependencies and getting the initial state setup for the test. For simple tests, this step may only consist of creating the class that we’re testing. For more complicated tests, there may be more dependencies to create, some methods to call, and possibly modifying the local environment. The Act step is where the method or property that we’re testing is called. This step should be the simplest portion of the test since it should only be a line or two of code. The third and final step is the Assert step where we check that the result we observed was correct. For simple tests, this could be a single line where check a value whereas more complicated tests may need to check various properties.

Using WhenMovingForward as an example, here’s what an example test might look like.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
using NUnit;
using System;

namespace MarsRover.UnitTests.RoverTests
{
  [TestFixture]
  public class WhenMovingForward()
  {
    [Test]
    public void AndFacingNorthThenYIncreasesByOne()
    {
      // Arrange
      var rover = new Rover { Orientation = Direction.North };
      var initialLocation = rover.Location;

      // Act
      rover.MoveForward();

      // Assert
      var expectedLocation = new Coordinate {X=initialLocation.X, Y=initialLocation.Y+1};
      Assert.AreEqual(expectedLocation, initialLocation);
    }
  }
}

I like to think of Arrange/Act/Assert (AAA) as a science experiment because we have to first find a hypothesis, design some test to prove or disprove the hypothesis, get all the necessary ingredients together for the test, run the test, and see if we have evidence to support our hypothesis.

Wrapping Up

In this post, we took a brief break from the Mars Rover kata to get a quick understanding of unit testing and the naming conventions we’ll be leveraging for the rest of the series! We first talked about the importance of naming and how if we follow Given, When, Then syntax, we can turn our user stories into readable test names for future engineers. From there, I showed you Arrange, Act, Assert notation for tests, and an example test using the convention. Next time, we’ll start implementing Rover!

Additional Resources

Mars Rover - Defining the Problem

In this installment, we’ll be looking at the problem description for Mars Rover. After becoming more familiar with the problem, we’ll start by identifying the terminology that we should be using when talking about the problem by defining a ubiquitous language. From there, I’ll show you how to break down the problem into various concepts and how to find the relationships between the various pieces. By the end of this post, you should feel comfortable exploring a new domain, understanding the terminology used, and defining relationships.

Problem Description

Congratulations and welcome to the S.P.A.C.E¹ Institute, good to have you aboard! Our big focus for the year is to develop a rover that can navigate the surface of Mars! While the engineers are working on the design and building of the rover, we can focus on building the navigation module and start iterating on its design. With that in mind, here are a couple of assumptions we’re going to make for this version.

  1. The rover will be traveling on a two-dimensional plane that should be modeled as a coordinate (X, Y)
  2. The rover is guaranteed to be able to travel in a chosen direction (no worries about obstacles or other landmarks)

Given the above assumptions, here are the business rules that the emulation will need to follow

  • When the emulation starts, the rover will always be at (0, 0) and facing North
  • There are a series of commands that the rover can receive that can change its location or direction
    • When the rover is told to move forward, then it will move one rover unit in the direction it’s facing
    • When the rover is told to move backward, then it will move rover unit away from the direction it’s facing
    • When the rover is told to turn left, it will rotate 90 degrees to the left, but not change its location
    • When the rover is told to turn right, it will rotate 90 degrees to the right, but not change its location
    • When the emulation is told to quit, the rover will stop receiving commands
  • For the emulation, valid directions include North, East, South, and West
  • In order to help troubleshoot failures with the emulation, every time a command is received, both the command received, the rover’s location, and the rover’s orientation should be logged.

¹ Simple Programming Application Checks Expertise

Identifying the Domain

When building software, I want to understand the various business terms that are being used to describe the problem so that when I’m talking to subject matter experts (SMEs), I’m using the same terminology as they are. For those who are familiar with Domain-Driven Design, this practice of using the same terminology is known as defining a ubiquitous language and the goal is to make sure that when someone says Command, then we are all referring to the same concept. If you’ve ever worked in a codebase where something was called one thing, but the business referred to it as something different, then you are familiar with the pain of having to map between the two concepts.

Find The Nouns

When working to define the ubiquitous language, a common approach is to find the nouns that are being used in the description as this can create the foundation of your classes (if following Object-Oriented principles) or your types (if following Functional Programming principles).

Looking over the description again, these nouns stood out to me:

Identifies domain (Rover, Command, Location, Direction, and Orientation)

Domain models: Rover, Command, Location, Direction, and Orientation

Find The Relationships

Once the nouns have been found, I’ll pivot to finding out how these different concepts are related to each other. One approach to finding these relationships is using “has-a” and “is-a” relationships. At a high level, if two things are related through “has-a”, then those concepts should be composed together. If two concepts have an “is-a” relationship, then I know that the concepts should be interchangeable for one another.

To help identify these relationships, I would work with the SME to understand what each of these concepts means and how they relate to each other. Since it’ll be a bit hard to simulate a conversation, here’s the information that we would learn from our SME.

  • A Rover has a Location and an Orientation
  • Orientation is the Direction that a Rover is facing
  • Location is the coordinates that the Rover is located at
  • A Command is something that a Rover receives from the User
  • A Direction can be North, East, South, or West
  • A Command can be Move Forward, Move Backward, Turn Left, Turn Right, or Quit

With this new understanding, our concepts and relationships would look something like this:

Domain model for Mars Rover

Domain model relationships where Rover has a Location and an Orientation. Orientation is a Direction and Command is not related to anything.

Wrapping Up

In this post, we explored the problem description for the Mars Rover kata and built up our understanding of the various concepts by exploring the nouns. After finding the nouns, we leveraged “has-a” and “is-a” thinking to come up with a rough idea of how the various concepts related to one another. In the next post, we’ll be focusing on how to model these concepts in code!

Learning Through Example – Mars Rover Kata

Over my career, I’ve spent a lot of time bringing new engineers up to speed on how to break down problems, how to write better code, and how to write automated tests. What I’ve come to find out is that there are a ton of resources on how to do each of these things individually, but not very many that brings all of these concepts together in one place. The purpose of this series is to bring all of these ideas together as we solve the Mars Rover kata.

Purpose

For new engineers, this kata has about the right amount of complexity to explore these various concepts and help identify things to improve on. As a team lead at SentryOne, I’ve onboard interns and our associate engineers using this kata as a launch point to teach them the tooling and processes we use. In addition, this kata serves as a method to evaluate their current skills so we can help round out a solid foundation for their career.

By the end of this series, you will have a better understanding of how to break down a problem into small, deliverable pieces while being able to write tests around new functionality. Even though there will be a lot of code to look at, none of these concepts are technology-specific so I challenge you to follow along with your tech stack of choice and see how you would implement some of these concepts in that stack.

Technologies Used

On the topic of technologies, I’ll be using the following for my implementation of this kata. As long as you find the relevant tool for your technology stack, you should be able to follow along!

Series

Mars Rover - Modeling Concepts

In the last post, we took a look at the problem description for Mars Rover and developed a set of concepts for the problem. From these concepts, we were able to develop common terminology and determine the relationships between the concepts. In this post, I’m going to show how I think about software design in general and how to apply them when modeling in code.

As a note, I’ll be showing code in both C# (for Object-Oriented approaches) and F# (for Functional Programming approaches). Once again, these concepts are fundamentals, but depending on your technology stack, the implementations will vary.

Design Guidelines

When I’m designing my models, my end goal is to produce software that captures the problem at hand using the same terms that the business uses. By striving for this goal, I can have richer conversations with my stakeholders when I run into interesting interactions of various business rules and can speak to them using the right terminology. In addition to capturing the problem, I will focus on designing my models in such a way that a developer can’t violate a business rule because the code won’t compile. At this point, I would have made illegal states unrepresentable in my code.

I first came across this term while reading Scott Wlashcin‘s work on the terrific F# for Fun and Profit website and it immediately resonated with me. I’ve definitely been bitten before working in a codebase where I wrote some code that compiled but blew up in my face during runtime because the parameter I passed in wasn’t valid for the method I was calling. Wouldn’t it be nice if the compiler told me while I was writing the code that what I was doing wouldn’t work? By thinking a bit more about the models being used and what some of their properties are, we can make this goal achievable.

Modeling Types

With these goals in mind, when it comes to modeling concepts, I naturally gravitate to types and find that types will fall in one of three categories.

The Type Has a Finite Number of Values

If the type has a finite number of valid values, then we can remove error conditions by defining the type to only be one of those possible options. For those from an Object-Oriented background, enums are a great example of modeling these types as you can explicitly set a label for the different values. For those from a Functional background, sum types are a great way to model these choices.

Some examples of such a type include the states in the U.S., the suits for a deck of playing cards, or the months in a year.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
public enum State
{
  Alabama, Alaska, California, Delaware,
  Florida, Georgia, Tennessee, Wyoming
}

public enum Suit
{
  Hearts, Clubs, Spades, Diamonds
}

public enum Months
{
  January, February, March, April, May, June,
  July, August, September, October, November, December
}

1
2
3
type State = Alabama  | Alaska  | California
           | Delaware | Florida | Georgia
           | Tennesee | Wyoming

The Type Has an Infinite Number of Values

For other types, however, there are so many possible valid values that it’s impossible to list all of them. For example, if we were looking at valid house numbers for an address, any positive integer would be valid so good luck on defining every positive number as an enum or sum type.

In these cases, I will leverage built-in primitives to model the concept at first. So in the case of HouseNumber, an integer might be a good enough spot to start. However, if I then find myself writing code that can work on integers, but shouldn’t work on HouseNumbers, then I might wrap a stronger type around the integer (see below).

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
// If the value isn't a major component of the design, we can use a primitive type
int houseNumber;

// However, if the type is a major concept to the domain at hand,
// it makes sense to lift it to its own type
public class HouseNumber
{
  public int Value {get;}
  public StreetNumber(int input)
  {
    // validation logic
    Value = input;
  }
}

// The difference between the two approaches is that in the first case, this would work
int houseNumber = 400;
Math.Sqrt(houseNumber);

// But this wouldn't
var houseNumber = new HouseNumber(400);
Math.Sqrt(houseNumber); // fails to compile with "cannot convert from HouseNumber to double"

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
// If the value isn't a major component of the design, we can use a primitive type
let houseNumber:int;

// However, if the type is a major concept to the domain at hand,
// it makes sense to lift it to its own type (single case sum type)
type HouseNumber = HouseNumber of int

// The difference between the two approaches is that in the first case, this would work
let houseNumber = 400;
Math.Sqrt(houseNumber);

// But this wouldn't
let houseNumber = HouseNumber 400
Math.Sqrt(houseNumber); // fails to compile with
                        // "This expression was expected to have type 'float' but here has type 'HouseNumber'"

The Type Is a Composition of Other Types

As the saying goes, large programs are built by composing a bunch of smaller programs, and types are no different. As we begin to model more complicated types, it’s natural to start thinking about types being composed of other types. For these types, we’ll leverage either objects (if following OO) or records (if following FP).

One way you can determine if you’re needing a composite type like this is if you find yourself using the word and or has when describing the type, then it’s a composition. For example:

An Address has a HouseNumber, it has a StreetName, it has a State.

An Address consists of a HouseNumber and a StreetName and a State

1
2
3
4
5
6
public class Address
{
  public int HouseNumber {get; set;}
  public string StreetName {get; set;}
  public State State {get; set;}
}

1
2
3
4
5
type Address = {
  houseNumber:int,
  streetName:string,
  state:State
}

Modeling Types

Now that we’ve talked about some different modeling techniques, let’s see how we can apply those rules as we start to model Mars Rover. From the previous post, we were able to derive the following concepts and relationships:

  • A Rover has a Location and an Orientation
  • Orientation is the Direction that a Rover is facing
  • Location is the coordinates that the Rover is located at
  • A Command is something that a Rover receives from the User
  • A Direction can be North, East, South, or West
  • A Command can be Move Forward, Move Backward, Turn Left, Turn Right, or Quit

Yielding the following graph

Domain model for Mars Rover

Domain model relationships where Rover has a Location and an Orientation. Orientation is a Direction and Command is not related to anything.

Given the above rules, we can start taking a look at how to model these in code! We’ll first start with the models that don’t have a dependency, and then build up from there

Modeling Direction

From the above requirements, Direction can only be one of four possible values (North, East, South, West). So based on that, it looks like we can leverage the first rule and model Direction like so:

1
2
3
4
public enum Direction
{
  North, South, East, West
}

1
type Direction = North | East | South | West

Modeling Command

From the above requirements, Command can only be one of five possible values (MoveForward, MoveBackward, TurnLeft, TurnRight, and Quit). Based on that, we can once again leverage the first rule and model Command like so:

1
2
3
4
5
6
public enum Command
{
  MoveForward, MoveBackward,
  TurnLeft, TurnRight,
  Quit
}

1
2
type Command = MoveForward | MoveBackward
             | TurnLeft | TurnRight | Quit

Modeling Location

After talking more with our Subject Matter Expert, a Location is the Coordinate where the Rover is located.

Aha! A new concept!

When we ask additional questions, we find out that a Coordinate refers to the Cartesian Coordinate System and for the problem we’re solving, we can assume that a Coordinate represents two numbers where the first number represents the location from the x-axis and the second number represents the location from the y-axis.

With this new information, our mental model has changed to be the following

Domain model for Mars Rover

Domain model relationships where Rover has a Location and an Orientation. Location is a Coordinate where Coordinate has an X and Y value. Orientation is a Direction and Command is not related to anything.

Going into further discussion, we find out that both X and Y will be whole numbers for our emulation and that they can be negative. Based on these properties, it sounds like X and Y can be modeled as integers and therefore fall under the second rule.

Given that a Coordinate has to have both an X and Y value, it sounds like Coordinate falls under the third rule and that this concept is a composition of X and Y.

1
2
3
4
5
public class Coordinate
{
  public int X {get; set;}
  public int Y {get; set;}
}

1
type Coordinate = {x:int; y:int}

Modeling Orientation

From the above requirements, it seems like Orientation is what we call the Direction that the Rover is facing. Based on that, this sounds like a property that Rover would have.

Modeling Rover

Now that we have both the Direction and Coordinate concepts designed, we can start designing Rover. From the requirements, it looks like Rover is a combination of Direction (known as Orientation) and a Coordinate (known as a Location). Based on that, Rover falls under the third rule and looks like the following.

1
2
3
4
5
public class Rover
{
  public Direction Orientation {get; set;}
  public Coordinate Location {get; set;}
}

1
2
3
4
type Rover = {
  orientation:Direction;
  location:Coordinate;
}

Wrapping Up

In this post, we implemented the basic types needed to solve the Mars Rover kata! We first started by taking a look at the concepts identified earlier and thought about the characteristics of the type which helped guide us to build software that both uses the terms of the problem domain and also prevents us from creating errors by making illegal states unrepresentable. In the next post, we’ll start adding functionality to our application.

Additional Reading

My Interviewing Strategy

I’ve found that interviewing for a new job can be super stressful and it reminds me of speed dating, “Let’s get to know each other over the next few hours to see if this relationship can work”. With such a short time window, there’s not much time to ask “fluff” questions like “if you were a tree, what kind of tree would you be”. Instead, I’m more likely to ask some of the following questions to get a better understanding of what I’m about to step into. If the company can’t answer some of these questions, it’s not a deal breaker, but can be a red flag about this place.

With that being said, I hope these questions help you during your interviewing process!

Business Strategy

  • What is your biggest concern for ?
  • What was a major success for over the past year?
  • What was a stumbling block for over the past year?
  • What does your ideal customer look like?
  • What is your business model (i.e. how does make its money)?
  • Any thoughts of expanding to other markets (such as, if the company sells a particular type of medical device, are there any thoughts of making other devices or add-ons for the main device)? If not, why?
  • Who would you say are your biggest competitors? What differentiates you from them?

Software Development

  • What is the current architecture of the software?
  • What is the direction that is moving to?
  • What is your current tech stack?
  • What development methodologies (TDD, Pairing, Mobbing, XP, etc…) are you using?
  • How do you maintain quality?
  • What is one quality you appreciate it a teammate?
  • What is one quality that gets on your nerves? How much work is new (greenfield) vs maintenance (brownfield)?

General

  • What would you consider to be a big success over the past year?
  • What would you consider a failure or stumbling block over the past year?
  • What is the best thing about working at ?
  • What is one thing that be improved about working at ?
  • What determines “success” for this role? How would you measure / know it?
  • What does a typical day look like for this role?

Benefits

  • How many vacation days? How many sick days? Are they from the same bucket (i.e PTO) or different buckets?
  • Is there a 401(K)? If so, what’s the vesting period (i.e. how long until the employer contributions become yours)? What’s the employer contribution?
  • Is there a training budget? If so, is it per person, per team, per department? What do you typical training expenses look like (books, videos, conferences, or in person training)?
  • When does open enrollment begin for insurance? Am I covered on day one or is there a waiting period?

TIL – How To Check If a Substitute Was Called Zero Times

Setup

During this past week, I’ve been working on a new feature and during development, I ended up with code that looked like this:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
public class PermissionChecker
{
  public PermissionChecker(IModuleDisabler moduleDisabler, User user)
  {
      if (user.IsAdmin) return;
      else if (user.HasFullRights) ConfigureFullRights(moduleDisabler);
      else if (user.HasPartialRights) ConfigurePartialRights(moduleDisabler);
  }

  private void ConfigureFullRights(IModuleDisabler disabler)
  {
      disabler.DisableSystemAdminModule();
  }

  private void ConfigurePartialRights(IModuleDisabler disabler)
  {
      disabler.DisableSystemAdminModule();
      disabler.DisableReportModule();
      disabler.DisableUserManagementModule();
  }
}

So the code is pretty straight forward, I have a PermissionChecker whose job is to use the IModuleDisabler to turn off certain modules depending upon the user permissions. Pretty straightforward implementation.

Now that the solution is fleshed out, it’s time to write some tests around this. When it comes to testing classes that have dependencies on non-trivial classes, I use NSubstitute, a mocking tool, to create mock versions of those dependencies. In this case, NSubstitute allows me to test how the IModuleDisabler is being used by the PermissionsChecker.

For example, let’s say that I wanted to test how the PermissionChecker interacts with the IModuleDisabler when the user has a partial access, I’d write a test that looks like the following:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
[Test]
public void And_the_user_has_partial_access_then_the_disabler_disables_the_report_module()
{
     // Arrange
     var permissionChecker = new PermissionChecker();
     var mockDisabler = Substitute.For();
     var user = new User {HasPartialAccess = true};

     // Act
     permissionChecker.CheckPermissions(mockDisabler, user);

     // Assert
     mockDisabler.Received(1).DisableReportModule();
}

In the above test, our assertion step is to check if the mockDisabler received a single call to the DisableReportModule. If it didn’t receive a call, then the test fails. We can write similar tests for the different modules that should be disabled for the partial rights permission and follow a similar pattern for the full rights permission.

However, things get a bit more interesting when we’re testing what happens if the user is an admin. If we follow the same pattern, we’d end up with a test that looks like this:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
[Test]
public void And_the_user_has_admin_permissions_then_the_disabler_is_not_used()
{
     // Arrange
     var permissionChecker = new PermissionChecker();
     var mockDisabler = Substitute.For();
     var user = new User {IsAdmin = true};

     // Act
     permissionChecker.CheckPermissions(mockDisabler, user);

     // Assert
     mockDisabler.DidNotReceive().DisableSystemAdminModule();
     mockDisabler.DidNotReceive().DisableReportModule();
     mockDisabler.DidNotReceive().DisableUserManagementModule();
}

This solution works for now, however, there is a major maintainability issue, can you spot it?

Problem

The issue arises when we add a new module to be disabled which forces the IModuleDisabler to implement a new method. In that case, you need to remember to update this test to also check that the new method wasn’t being called. If you forget, this test would still pass, but it’d pass for the wrong reason.

To help illustrate, let’s say that another method, DisableImportModule, has been added to the IModuleDisabler interface. In addition, we also need to make sure that this is called when users have partial access, but should not be called for users who are admins or users who have full access.

To fulfill those requirements, we modify the PermissionChecker as so:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
public class PermissionChecker
{
  public PermissionChecker(IModuleDisabler moduleDisabler, User user)
  {
      if (user.IsAdmin) return;
      else if (user.HasFullRights) ConfigureFullRights(moduleDisabler);
      else if (user.HasPartialRights) ConfigurePartialRights(moduleDisabler);
  }

  private void ConfigureFullRights(IModuleDisabler disabler)
  {
      disabler.DisableSystemAdminModule();
  }

  private void ConfigurePartialRights(IModuleDisabler disabler)
  {
      disabler.DisableSystemAdminModule();
      disabler.DisableReportModule();
      disabler.DisableUserManagementModule();
      disabler.DisableImportModule();
  }
}

At this point, we’d write another test for when the a user has partial access, the import module should be disabled. However, it’s very unlikely that we’d remember to update the test for the admin. Remember, for the admin, we’re checking that it received no calls to any disable methods and the way we’re doing that is by checking each method individually.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
[Test]
public void And_the_user_has_admin_permissions_then_the_disabler_is_not_used()
{
  // Arrange
  var permissionChecker = new PermissionChecker();
  var mockDisabler = Substitute.For();
  var user = new User {IsAdmin = true};

  // Act
  permissionChecker.CheckPermissions(mockDisabler, user);

  // Assert
  mockDisabler.DidNotReceive().DisableSystemAdminModule();
  mockDisabler.DidNotReceive().DisableReportModule();
  mockDisabler.DidNotReceive().DisableUserManagementModule();
  // Need to add check for DidNotReceive().DisableImportModule();
}

Solution

There’s got to be a better way. After some digging around, I found that any NSubstitute mock, has a ReceivedCalls method that returns all calls that the mock received. With this new knowledge, we can refactor the previous test with the following:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
[Test]
public void And_the_user_has_admin_permissions_then_the_disabler_is_not_used()
{
  // Arrange
  var permissionChecker = new PermissionChecker();
  var mockDisabler = Substitute.For();
  var user = new User {IsAdmin = true};

  // Act
  permissionChecker.CheckPermissions(mockDisabler, user);

  // Assert
  CollectionAssert.IsEmpty(mockDisabler.ReceivedCalls());
}

This solution is much better because if we add more modules, this test is still checking to make sure that admin users do not have any modules disabled.

Summary

When using a NSubstitute mock and you need to make sure that it received no calls to any methods or properties, you can using NSubstitute’s ReceivedCalls in conjunction with CollectionAssert.IsEmpty to ensure that the substitute was not called.

Using F# To Solve a Constraints Problem

In this post, I’m going to solve a logic puzzle using C# and F#. First, I’ll define the problem being solved and what our restrictions are. Next, I’ll show how I’d break down the problem and write an easy-to-read, extendable solution using idiomatic C#. Afterwards, I’ll solve the same problem and write an easy-to-read, extendable solution writing in idiomatic F#. Finally, we’ll compare the two solutions and see why the F# solution is the better solution.

The Problem

For this problem, I’m going to write a constraint solver (thanks to Geoff Mazeroff for the inspiration).

If you’re not familiar with the concept, a constraint is simply some rule that must be followed (such as all numbers must start with a 4). So a constraint solver is something that takes all the constraints and a source of inputs and returns all values that fit all the constraints.

With that being said, our source will be a list of positive integers and our constraints are the following:

  • 4 digits long (so 1000 – 9999)
  • Must be even (so 1000, 1002, 1004, etc…)
  • The first digit must match the last digit (2002, 2012, 2022, etc…)

To further restrict solutions, all code will be production ready. This includes handling error conditions (like input being null), being maintainable (easily adding more constraints) and easy to read.

To quickly summarize, we need to find a robust, maintainable, and readable solution to help us find all 4 digit number that are even and that the first and last digit are equal.

Implementing a Solution in C

For the C# solution, I’m going to need a class for every constraint, a class to execute all constraints against a source (positive integers) and a runner that ties all the pieces together.

Starting with the smaller blocks and building up, I’m going to start with the constraint classes. Each constraint is going to take a single number and will return true if the number follows the constraint, false otherwise.

With that being said, I’d first implement the constraint that all numbers must be 4 digits long

1
2
3
4
5
6
7
class MustBeFourDigitsLongConstraint
{
    public bool FollowsConstraint(int value)
    {
        return value.ToString().Length == 4;
    }
}

Second, I’d write the constraint that all numbers must be even

1
2
3
4
5
6
7
class MustBeEvenConstraint
{
    public bool FollowsConstraint(int value)
    {
        return value % 2 == 0;
    }
}

Third, I’d implement the constraint that all numbers must have the same first digit and the last digit

1
2
3
4
5
6
7
8
class FirstDigitMustEqualLastDigitConstraint
{
    public bool FollowsConstraint(int value)
    {
        var valueString = value.ToString();
        return valueString[0] == valueString[valueString.Length-1];
    }
}

At this point, I have the constraints written, but I need them to follow a general contract so that the Constraint Solver (about to be defined) can take a list of these constraints. I’ll introduce an interface, IConstraint and update my classes to implement that interface.

1
2
3
4
5
6
7
8
9
public interface IConstraint
{
    bool FollowsConstraint(int value);
}
class MustBeFourDigitsLongConstraint : IConstraint {/* Implementation Details Omitted */}

class MustBeEvenConstraint : IConstraint {/* Implementation Details Omitted */}

class FirstDigitMustEqualLastDigitConstraint : IConstraint {/* Implementation Details Omitted */}

So now I have the constraints defined and they’re now implementing a uniform interface, I can now create the constraint solver. This class is responsible for taking the list of numbers and the list of constraints and then returning a list of numbers that follow all constraints.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
class ConstraintSolver
{
    public List FindValues(List constraints, List values)
    {
        if (constraints == null) throw new ArgumentNullException("constraints");
        if (values == null) throw new ArgumentNullException("values");

        var result = values;
        foreach (var constraint in constraints)
        {
            result = result.Where(x => constraint.FollowsConstraint(x)).ToList();
        }
        return result;
    }
}

Finally, I can put all the pieces together using LINQPad (Full C# solution can be found here).

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
void Main()
{
    var numbers = Enumerable.Range(0, 10000).ToList();
    var constraints = new List<IConstraint>{new MustBeFourDigitsLongConstraint(), new MustBeEvenConstraint(), 
             new FirstDigitMustEqualLastDigitConstraint()};

    var constraintSolver = new ConstraintSolver();
    var result = constraintSolver.FindValues(constraints, numbers.ToList());

    result.Dump();
}

This solution is easily extendable because if we need to add another constraint, we just add another class that implements the IConstraint interface and change the Main method to add an instance of the new constraint to the list of constraints.

Implementing a Solution in F

Now that we have the C# solution, let’s take a look at how I would solve the problem using F#.

Similar to the C# solution, I’m going to create a function for every constraint, a function to execute all constraints against a source (positive integers) and a runner that ties all the pieces together.

Also similar to the C# solution, I’m going to start with creating the constraints and then work on the constraint solver function.

First, I’d implement that the number must be four digits long constraint.

1
2
let mustBeFourDigit number = 
    number.ToString().Length = 4

Next, the number must be even constraint.

1
2
let mustBeEven number =
    number % 2 = 0

Lastly, the first digit is the same as the last digit constraint.

1
2
3
4
5
let firstDigitMustBeEqualLast number =
    let numberString = number.ToString().ToCharArray()
    let firstDigit = numberString.GetValue(0)
    let lastDigit = numberString.GetValue(numberString.Length-1)
    firstDigit = lastDigit

At this stage in the C# solution, I had to create an interface, IConstraint, so that the constraint solver could take a list of constraints. What’s cool with F# is that I don’t have to define the interface. The F# type inference is saying that each of these functions are taking the same input (some generic `a) and returning a bool, so I can add all of them to the list. This is pretty convenient since I don’t have to worry about this piece of plumbing.

Now that the different constraints are defined, I’d go ahead and write the last function that takes a list of constraints and a list of numbers and returns the numbers that the constraints fit. (Confused by this function? Take a look at Implementing your own version of # List.Filter)

1
2
3
4
5
6
let rec findValidNumbers numbers constraints = 
    match constraints with
    | [] -> numbers
    | firstConstraint::remainingConstraints ->
        let validNumbers = numbers |> List.filter firstConstraint
        findValidNumbers validNumbers remainingConstraints

Finally, all the pieces are in place, so I can now put all the pieces together using LINQPad.

1
2
3
4
5
6
let numbers = [1000 .. 9999]
let constraints = [mustBeFourDigits; mustBeEven; firstDigitMustEqualLast;]

let result = findValidNumbers numbers constraints

printf "%A" result

Comparing Both Solutions

Now that we have both solutions written up, let’s compare and see which solution is better.

First, the same design was used for both solutions. I decided to use this design for both because it’s flexible enough that we could add new constraints if needed (such as, the 2nd digit must be odd). As an example, for the C# solution, I’d create a new class that implemented IConstraint and then update the runner to use the new class. For the F# solution, I’d create a new function and update the runner. So, I’d think it’s safe to say that both solutions score about the same from a maintainability and extendability point of view.

From an implementation perspective, both solutions are production ready since the code handles possible error conditions (C# with null checks in the ConstraintSolver class, F# with none because it doesn’t support null). In addition to being robust, both solutions are readable by using ample whitespace and having all variables, classes, and interfaces clearly described.

With that being said, this is where the similarities end. When we look at how much code was written to solve the problem, we have a stark difference. For the C# solution, I ended up with 48 lines of code (omitting blank lines), however, for the F# solution, it only took 19. Now you could argue that I could have written the C# solution in fewer lines of code by removing curly braces around one line statements or ignoring null inputs. However, this would lead the code to be less robust.

Another difference between the F# solution and C# is that I was able to focus on the solution without having to wire up an interface. You’ll often hear developers talk about the how little plumbing you need for F# to “just work” and this small example demonstrates that point.

Another difference (albeit subtle) is that the F# solution is that I can use the findValidNumbers function with any generic list of values and any generic list of functions that take the generic type and return true/false.

In other words, if I had another constraint problem using strings, I’d still implement the different constraints, but I could use the same findValidNumbers function. At that point, however, I’d probably rename it to findValidValues to signify the generic solution.

What’s awesome about this is that I didn’t have to do any more work to have a generic solution, F# did that for me. To be fair, the C# solution can easily be made generic, but that would have to be a conscious design decision and I think that’s a downside.

TL;DR

In this post, we took a look at solving a number constraint problem by using idiomatic C# and F#. Even though both solutions are easy to read and easy to extend, the F# version was less than 1/2 the size of the C# solution. In addition, I didn’t have to do any plumbing for the F# version, but had to do some for the C# solution, and to top it off, the F# solution is generically solved, whereas the C# solution is not.

Implementing Your Own Version of F#’s List.Filter

As I’ve been thinking more about F#, I began to wonder how certain methods in the F# stack work, so I decided to implement F#’s List.filter method.

For those of you who aren’t familiar, List.Filter takes a function that returns true or false and a list of values. The result of the call is all values that fulfill the fuction.

For example, if we wanted to keep just the even numbers in our list, then the following would accomplish that goal.

1
2
3
4
5
6
let values = [1;2;3;4]
let isItEven x = x % 2 = 0


let evenValues = List.filter isItEven values
// val it : int list = [2; 4]

Now that we know the problem, how would we begin to implement? First, we need to define a function called filter:

1
let filter () =

However, to match the signature for List.filter, it needs to take a function that maps integers to bools and the list of values to work on

1
let filter (func:int->bool) (values:int List) =

Now that we have the signature, let’s add some logic to match on the list of values. When working with lists, there are two possibilities, an empty list and a non-empty list. Let’s first explore the empty list option.

In the case of an empty list of values, then it doesn’t matter what the func parameter does, there are no possible results, so we should return an empty list for the result.

1
2
3
let filter (func:int->bool) (values:int List) =
    match values with
    | [] -> []

Now that we’ve handled the empty list, let’s explore the non-empty list scenario. In this branch, the list must have a head and a tail, so we can deconstruct the list to follow that pattern.

1
2
3
4
let filter (func:int->bool) (values:int List) =
    match values with
    | [] -> []
    | head::tail -> // what goes here?

Now that we’ve deconstructed the list, we can now use the func parameter with the head element. If the value satisfies the func parameter, then we want to add the head element to the list of results and continue processing the rest of the list. To do that, we can use recursion to call back into filter with the same func parameter and the rest of the list:

1
2
3
4
5
let rec filter (func:int->bool) (values:int List) =
    match values with
    | [] -> []
    | head::tail -> 
         if func head then head :: filter func tail

At this point, we need to handle the case where the head element does not satisfy the func parameter. In this case, we should not add the element to the list of results and we should let filter continue the work

1
2
3
4
5
6
let rec filter (func:int->bool) (values:int List) =
    match values with
    | [] -> []
    | head::tail -> 
         if func head then head :: filter func tail
         else filter func tail

By handling the base case first (an empty list), filter can focus on the current element in the list (head) and then recurse to process the rest of the list. This solution works, but we can make this better by removing the type annotations. Interestingly enough, we don’t care if we’re working with integers, strings, or whatever. Just as long as the function takes some type and returns bool and the list of values matches the same type as the func parameter, it works. So then we end up with the following:

1
2
3
4
let rec filter func values =
    match values with
    | [] -> []
    | head::tail -> if func head then head :: filter func tail else filter func tail

In general, when working with lists, I tend to start by matching the list with either an empty list or non-empty. From there, I’ve got my base case, so I can focus on the implementation for the first element. After performing the work for the first element, I can then recurse to the next element.

Today I Learned – The Chain of Responsibility Design Pattern

There is nothing new in the world except the history you do not know. – Harry S. Truman

The more experience I gain problem solving, the more this holds true. For this post, I’m going to first discuss the problem that I was trying to solve. Next, I’ll show what my first solution was, followed by the shortcomings of this solution. Thirdly, we’ll iterate over a better solution to the problem. This in turn, will provide the motivation for what the Chain of Responsibility is and how to implement. Finally, I’ll wrap up with what the benefits were of using this design. .

Problem I was trying to solve

As part of the process of installing our software, there are scripts that will update the database from it’s current version to the latest version. As it stands, it needs to be able to upgrade a database from any version to the current version.

Previous Solution

The first thing that comes to me is that I need to apply database scripts in a sequential way. For example, if the database’s current version is 1.0 and the latest version is 3.0, it would need to apply the script to upgrade the database from 1.0 to 2.0 and then apply the script to upgrade the database from 2.0 to 3.0.

For the first implementation, there were only two versions, 1.0 and 2.0. Since I didn’t want to build in a lot of functionality if it wasn’t needed yet, I created a helper method that returns the correct updater for a given version. In the below code, if the version does not exist, I assume the database does not exist and return the class that will create the database. Otherwise if the version is 1.0, I return a class that is responsible for the upgrading a database from 1.0 to 2.0. If the version is 2.0, I return a class that doesn’t do anything (i.e. there’s no upgrades to be done).

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
public IDatabaseUpdater GetDatabaseUpdater(string version)
{
  if (string.IsNullOrWhiteSpace(version))
    return new DatabaseCreator();
  if (version == "1.0")
    return new Database100To200Updater();
  if (version == "2.0")
    return new CurrentVersionUpdater();
  throw new ArgumentException("The version " + version + " is not supported for database upgrades.");
}

Problem with solution

This solution worked well when there only three possible actions (create a new database, apply the single script, or do nothing). However, we are now going to be shipping version 3.0 and there will need to be a new class that is responsible for upgrading the 2.0 to 3.0. In order to add this functionality, I’d have to do the following:

  1. Create the Database200To300Updater class that contained the logic for updating the database from 2.0 to 3.0.
  2. Modify the Database100To200Updater class to also use the Database200To300Updater in order to perform the next part of the upgrade.
  3. Add additional logic to the above method so that if the database is 2.0 to return the Database200To300Updater class.

After making the modifications, the method now looks like:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
public IDatabaseUpdater GetDatabaseUpdater(string version)
{
  if (string.IsNullOrWhiteSpace(version))
    return new DatabaseCreator();
  if (version == "1.0")
    return new Database100To200Updater(new Database200To300Updater());
  if (version == "2.0")
    return new Database200To300Updater();
  if (version == "3.0")
    return new CurrentVersionUpdater();

  throw new ArgumentException("The version " + version + " is not supported for database upgrades.");
}

So far, so good, we now have the logic to be able to apply scripts in order, however, now that we’ve added version 3.0, I start to wonder what I would do if we added more versions? After some thought, it would look identical to the previous steps (see below for what would happen if we added version 4.0).

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
public IDatabaseUpdater GetDatabaseUpdater(string version)
{
  if (string.IsNullOrWhiteSpace(version))
    return new DatabaseCreator();
  if (version == "1.0")
    return new Database100To200Updater(new Database200To300Updater(new Database300To400Updater()));
  if (version == "2.0")
    return new Database200To300Updater(new Database300To400Updater());
  if (version == "3.0")
    return new Database300To400Updater();
  if (version == "4.0")
    return new CurrentVersionUpdater();
  throw new ArgumentException("The version " + version + " is not supported for database upgrades.");
}

If we create some variables to hold onto these classes, and reorder the if statements, we can write this helper method as:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
public IDatabaseUpdater GetDatabaseUpdater(string version)
{
  if (string.IsNullOrWhiteSpace(version))
    return new DatabaseCreator();
  if (version == "4.0")
    return new CurrentVersionUpdater();
  var database300Updater = new Database300To400Updater();
  var database200Updater = new Database200To300Updater(database300To400Updater);
  var database100Updater = new Database100To200Updater(database200To300Updater);

  if (version == "1.0")
    return database100Updater;
  if (version == "2.0")
    return new database200Updater;
  if (version == "3.0")
    return new database300Updater;

  throw new ArgumentException("The version " + version + " is not supported for database upgrades.");
}

Motivation for the Chain of Responsibility

What I find interesting in this design is that I’ve now chained these updater classes together so that if the version 1.0 is returned, it will also use the 2.0 updater, which in turn calls the 3.0 updater. It was at this point, that I remembered a design pattern that followed this structure.

In this design pattern, you essentially have Handlers (in my case updaters) that check to see if they can handle the request. If so, they do and that stops the chain. However, if they can’t handle the request, they pass it to their Successor (which was also a Handler) to handle the request. The design pattern I was thinking about is the Chain of Responsibility pattern.

In order to implement this pattern, you need to have an IHandler interface that exposes a Handle method and either a method or property to set the Successor. The method is the action to take (in our case Update) and the Successor represents the next Handler in the chain if the request could not be handled. The second component is referred to as ConcreteHandlers and they are just the implementors of the interface. One way to implement this is like the following:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
public interface IHandler
{
  IHandler Successor { get; set; }
  void Update(int version);
}

public class ConcreteHandlerA : IHandler
{
  public IHandler Successor { get; set; }

  public void Update(int version)
  {
    if (CanTheRequestBeHandled) {
      // handle the request
    }
    else {
      Successor.Update(version);
    }
  }
}

The main difference between the pattern and what I need is that instead of doing if (canHandle)/else call Successor, what I’m really looking for is to run the upgrade script if the version we’re upgrading to is higher than our current version and then always call the successor. Given this change, here’s what that new implementation looks like:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
public class ConcreteHandlerA : IHandler
{
  public Successor { get; set; }
  public void Update(int version)
  {
    if (CanTheRequestBeHandled) {
      // handle the request
    }
    Successor.Update(version);
  }
}

Implementing the Chain of Responsibility

Now that I know the pattern to use and how it works, I need to update the IDatabaseUpdater interface to follow the IHandler interface. Next, I will need to modify the concrete handlers to use the new interface correctly.

Implementing the Handler

First, we will update our IDatabaseUpdater interface to follow the IHandler look:

Before
1
2
3
4
public interface IDatabaseUpdater
{
  void Update(int version);
}
After
1
2
3
4
5
public interface IDatabaseUpdateHandler
{
  void Update(int version);
  IDatabaseUpdateHandler Successor { get; set; }
}

Implementing the Concrete Handler

Second, we will need to update our concrete handlers to implement the interface correctly and to update their UpdateMethod to follow the design. In my case, the concrete handlers perform similar logic, so one of the classes is used for an example.

Before
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
public class Database100To200Updater : IDatabaseUpdater
{
  private Database200To300Updater _successor;
  public Database100To200Updater(Database200To300Updater successor)
  {
    if (successor == null)
      throw new ArgumentNullException("successor");
    _successor = successor;
  }

  public void Update()
  {
    Console.WriteLine("Updating the database to version 2.0");
    _successor.Update();
  }
}
After

Thanks to the public property, I was able to remove the private member and that in turn allowed me to remove the constructor. 

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
public class Database100To200Updater : IDatabaseUpdateHandler
{
  public void Update(int version)
  {
    if (version >= 2)
      Console.WriteLine("Updating the database to version 2.0");
    if (Successor != null)
      Successor.Update(version);
  }

  public IDatabaseUpdateHandler Successor { get; set;}
}

Updating the Helper Method

Now that we’ve updated the interface and implementors, it’s time to update the helper method to take advantage of the new design.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
public IDatabaseUpdateHandler GetDatabaseUpdater(string version)
{
  if (string.IsNullOrWhiteSpace(version))
    return new DatabaseCreator();

  var database300To400 = new Database300To400Updater();
  var database200To300 = new Database200To300Updater();
  var database100To200 = new Database100To200Updater();

  database100To200.Successor = database200To300;
  database200To300.Successor = database300To400;

  return database100To200;
}

Chain of Responsibility is great, here’s why

What I really like about the chain of responsibility pattern is that I was able to connect my upgrade classes together in a consistent fashion. Another reason why I like this pattern is that it forces me to have the logic to determine whether I should run the update or not inside the individual classes instead of the helper method. This produces more readable code which then lends itself to easier maintainability.

Today I Learned – How to Break Down A Massive Method

During this past week, I was working with our intern and showing him some cool debugging tricks when I came across a massive method. I gave him 30 seconds to look at it and tell me what he thought the method was doing. After a while, he was able to figure it out, but the fact that it wasn’t easy discernible was enough to give pause.

The lesson here is that if you can’t determine what the method is doing easily, then it’s probably doing way too much (violating the Single Responsibility Principle) and needs to be broken into more easily readable pieces.

To demonstrate what I mean, I wrote a program that inserts Messages into a database. A Message contains a description, the number (for identification when troubleshooting) and the module. We would have issues where different messages would have the same number which would cause confusion when troubleshooting errors.

In the program I wrote, the user provides the message and what module the message belongs to and the program automatically generates the message number and inserts the message into the database.

For brevity’s sake, shown below is the logic for determining what the next message number should be.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
public int GetNextAlertAndErrorModuleNumber(string module)
{
  if (String.IsNullOrEmpty(module))
    throw new ArgumentException("module cannot be null or empty");
  if (_connection == null)
    _connection = CreateConnection();

  var results = new List<int>();

  _connection.Open();
  var cmd = new SqlCommand("dbo.GetAlertData", _connection);
  cmd.CommandType = CommandType.StoredProcedure;

  var reader = cmd.ExecuteReader();
  while (reader.Read())
  {
    if (!reader["ALERT_ID_NUMBER"].ToString().Contains(module))
      continue;

    var pieces = reader["ALERT_ID_NUMBER"].ToString().Split(‘ ‘);

    results.Add(Int32.Parse(pieces[1]));
  }
  if (reader != null)
    reader.Close();

  cmd = new SqlCommand("dbo.GetErrorData";, _connection);
  cmd.CommandType = CommandType.StoredProcedure;

  reader = cmd.ExecuteReader();
  while (reader.Read())
  {
    if (!reader["ERROR_ID_NUMBER"].ToString().Contains(module))
      continue;

    var pieces = reader["ERROR_ID_NUMBER"].ToString().Split(‘ ‘);

    results.Add(Int32.Parse(pieces[1]));
  }
  if (reader != null)
    reader.Close();

  if (_connection != null)
    _connection.Close();

  return results.Max() + 1;
}

The method itself isn’t complex, just calling some stored procedures, parsing the output and adding the number to a list. However, it’s not abundantly clear what the purpose of the calling the stored procedures.

First, it looks like we’re reading the alerts error numbers from a stored procedure call, why don’t we extract that logic out to a helper method and have the public method call the helper?

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
public int GetNextAlertAndErrorModuleNumber(string module)
{
  if (String.IsNullOrEmpty(module))
    throw new ArgumentException(&amp;amp;quot;module cannot be null or empty&amp;amp;quot;);

  if (_connection == null)
    _connection = CreateConnection();

  var results = new List<int>();

  _connection.Open();
  results.AddRange(ReadAlerts(module.ToUpper()));

  var cmd = new SqlCommand("dbo.GetErrorData", _connection);
  cmd.CommandType = CommandType.StoredProcedure;

  var reader = cmd.ExecuteReader();
  while (reader.Read())
  {
    if (!reader["ERROR_ID_NUMBER"].ToString().Contains(module))
      continue;

    var pieces = reader["ERROR_ID_NUMBER&"].ToString().Split(‘ ‘);

    results.Add(Int32.Parse(pieces[1]));
  }
  if (reader != null)
    reader.Close();

  if (_connection != null)
    _connection.Close();

  return results.Max() + 1;
  }

  private List<int> ReadAlerts(string module)
  {
    var results = new List<int>();
    var cmd = new SqlCommand("dbo.GetAlertData", _connection);
    cmd.CommandType = CommandType.StoredProcedure;

    var reader = cmd.ExecuteReader();
    while (reader.Read())
    {
      if (!reader["ALERT_ID_NUMBER"].ToString().Contains(module))
        continue;

      var pieces = reader["ALERT_ID_NUMBER"].ToString().Split(‘ ‘);
      results.Add(Int32.Parse(pieces[1]));
    }
    if (reader != null)
    reader.Close();

    return results;
}

By doing this, we fix two issues at once. First, we’ve given a name to the process of reading the alerts which in turns allows us to quickly understand what the public method should be doing (i.e. improved readability).

Second, it allows us for easier debugging because we now have smaller components. For example, let’s say that we were getting the wrong value. In the first implementation, we would have to put breakpoints in different areas trying to determine which part was broken. However, in the new form, we can check to see if ReadAlerts is behaving correctly. If it isn’t, we now know the bug has to be in that method, otherwise, it’s in the rest.

For the next step, you may have noticed that we can repeat the same refactoring trick again, except this time, we can extract the logic for reading the errors into a helper method.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
public int GetNextAlertAndErrorModuleNumber(string module)
{
  if (String.IsNullOrEmpty(module))
    throw new ArgumentException("module cannot be null or empty");
  if (_connection == null)
    _connection = CreateConnection();

  _connection.Open();

  var results = new List<int>();
  results.AddRange(ReadAlerts(module.ToUpper()));
  results.AddRange(ReadErrors(module.ToUpper()));

  if (_connection != null)
    _connection.Close();

  return results.Max() + 1;
}

private List<int> ReadAlerts(string module)
{
  var results = new List<int>();
  var cmd = new SqlCommand("dbo.GetAlertData", _connection);
  cmd.CommandType = CommandType.StoredProcedure;

  var reader = cmd.ExecuteReader();
  while (reader.Read())
  {
    if (!reader["ALERT_ID_NUMBER"].ToString().Contains(module))
      continue;

    var pieces = reader["ALERT_ID_NUMBER"].ToString().Split(‘ ‘);
    results.Add(Int32.Parse(pieces[1]));
  }
  if (reader != null)
    reader.Close();

  return results;
}

private List<int> ReadErrors(string module)
{
  var results = new List<int>();
  var cmd = new SqlCommand("dbo.GetErrorData", _connection);
  cmd.CommandType = CommandType.StoredProcedure;

  var reader = cmd.ExecuteReader();
  while (reader.Read())
  {
    if (!reader["ERROR_ID_NUMBER"].ToString().Contains(module))
      continue;

    var pieces = reader["ERROR_ID_NUMBER"].ToString().Split(‘ ‘);
    results.Add(Int32.Parse(pieces[1]));
  }
  if (reader != null)
    reader.Close();

  return results;
}

After the changes, anyone who looks at the public API can easily see that it’s reading from both Alerts and Errors. This is really powerful because now you can communicate with non-technical people about requirements and what the code is doing.

Let’s say that in the future, the requirements change and this conversation plays out:

Alice (QA, finding an error) – Hey Bob, I was running one of our test plans and it looks like that we’re getting the wrong message number if we’re trying to add a new message and there are warning messages in the database. We are including the warning table when figuring that out, right?

Bob (Engineer, finding the root cause) – Hey you’re right, it looks like we’re only using the alerts and error tables when calculating the next number. Why don’t we write up a ticket for that and get a fix in?

The key point is that no matter how large a method is, there always have to be steps being performed in some order (by definition of an algorithm) and this is the key to refactoring larger methods into more maintainable pieces of code. The trick is determining what those steps are and making decisions on whether to make helper methods or helper classes.

If those steps become complicated, then they should be broken out into helper methods. As time progresses and those helper methods start to become more complicated, then those helper methods should in turn become classes of their own.