Invest in Dumb view controllers for the sake of your unit tests

William Boles -

Cohesion is important for your application. Each unit of work should have a clear purpose and clear area of responsibility, this is known as the Single Responsibility Principle (SRP). If you find that you are working on a method/class that doesn’t have clear responsibilities, when you get the chance you need to refactor that method/class into smaller pieces - this will produce a code base that should be easier to understand (maintain and extend).

With SRP in mind lets start thinking about our applications. A lot of focus is often given to the UI in our applications and it's very easy to get caught in the mindset of “the controller is king” and forget about SRP and MVC. But let's go the opposite way and instead only allow our view controllers to do the minimum amount and push that existing work to more focused classes. With this Dumb view controller approach the role of the view controller is to act as a piping/routing control that passes information between different (more focused) objects.

One of the main ideas behind MVC is to push the responsibility for an action as far down the chain as possible so ensuring that each object is only responsible for the minimum amount of actions. For example if we have a validation rule for a person’s telephone number, the view controller code to handle this could look like this:

- (void)textFieldDidEndEditing:(UITextField *)textField
{
    NSString *telephoneNumber = textField.text;

    if ([telephoneNumber length] != 12)
    {
        [self showErrorAlertViewWithErrorCode:148];
    }
    else
    {
        [self saveTelephoneNumber:telephoneNumber];
    }
}

Lets look at the unit test needed to test this method

- (void)testValidTelephoneNumber
{
    UTERegistrationViewController *viewController = [[UTERegistrationViewController alloc] init];

    UITextField *telephoneTextField = [[UITextField alloc] init];
    telephoneTextField.text = @"012345678912";

    [viewController textFieldDidEndEditing:telephoneTextField];

    //----Asserts here using call intercepting to determine if the test passes-----//
}

Hmm, let's take a moment to look over that unit test again. If we think about it we should be able to see that a valid telephone number doesn't have anything to do with a UITextField instance. However due to where our validation code is located we need to create a UITextField instance to also test validation. Now in MVC this business rule should not reside in the view controller but rather in the business (Model) class itself - UTETelephone. So let's get extracting:

- (BOOL)validTelephoneNumber:(NSString *)telephoneNumber error:(NSError **)error
{
   if ([telephoneNumber length] != 12)
   {
        if (error) 
        {
            NSMutableDictionary *userInfo = [NSMutableDictionary dictionary];

            [userInfo setObject:@"Telephone number must be at 12 characters in length" forKey:NSLocalizedDescriptionKey];

            *error = [NSError errorWithDomain:@"TelephoneValidation" code:148 userInfo:userInfo];
        }

        return NO;
    }
    else
    {
        return YES;
    }
}

The first thing you should notice is that the method has a lot more code than its equivalent in the view controller. Instinctively this feels wrong, you may even be thinking -

"Wait! Shouldn't we be reducing code not adding to it?"

Reducing the amount of code in your application is a noble aim however it needs to only be applied were by removing code we actually make it easier to understand what the method/class is attempting to do. So with that in mind, let's work out what we gained from the above extraction:

  • Produce a more complete business (Model) class
  • Data classes I feel are best avoided (let's chat about it later)
  • Easier to share telephone validation functionality
  • Say between a "registration form" and an "update your details form"
    • Only one method/class needs to be updated if what constitutes a valid telephone changes
  • Remove business rules from UI
  • No longer have to care about UITextField

All of the above, makes our unit tests easier to write.

- (void)testValidTelephoneNumber
{
    UTETelephone *telephone = [[UTETelephone alloc] init];

    NSError *error = nil;
    BOOL validTelephoneNumber = [telephone validTelephoneNumber:@"123456789101" error:&error];

    STAssertTrue(validTelephoneNumber, @"True should be returned for a valid telephone number");
}

Of course the above is only one unit test and we would need to write a few more to fully test this method. However we can see that there is no need for call intercepting to determine if the correct path for taken for an input. Instead we check what is returned (or set in the NSError's case) and allow UTERegistrationViewController to deal with connecting up the piping of information between different objects rather than the one method dealing with validation and piping.

A good indication of how well your application is designed is by how easy it is to write unit tests. If you find that you are spending a lot of time mocking out objects or setting up objects that are not directly under test (like the UITextField) - it may be a sign that you need to refactor your code.

In our applications we should always strive to make our view controllers dumb, as a dumb view controller means that we can separate our business rules from our UI implementation and so improve the quality of our unit tests.