Programmers: TDD your Legacy Code to Single Respnsibility Principle. Part 2 – The Glorious TDD Cycle

TL;DR

This article series shows the importance of having unit tests. It also shows the importance of adhering to the Single Responsibility Principle (SRP), as we get a new requirement for our code base. We take a section of “legacy code” and refactor it to more closely follow SRP, all while maintaining the safety net of unit tests and leveraging Test Driven Development (TDD). We also cover using Dependency Injection in this TDD context.

This was originally going to be one post, but ended up WAY too long, so it will be a series of three posts:
Part 1 – Setup and Planning the Responsibility Transfer
Part 2 – The Glorious TDD Cycle
Part 3 – Dependency Injection For the New Class and Wrap Up

Please following along on GitHub. The name of the downloaded .zip file for various links in this article correspond to commits in this Github repo.

In part 1, we planned the responsibilities we would transfer from GuessForm to the new QuestionAndAnswerValidator class, and did some other setup. Now we will start the TDD cycle to implement the new responsibilities on our new class all while maintaining unit test safety.

Test Driven Development (TDD) For The New Class We Will Extract – Happy Case

TDD is another concept discussed in Robert C. Martin’s Clean Code. In the discussion, he poses a method of writing production code, whereby you write a unit test that fails, and then you write the production code until the test passes. Though, don’t think you have to write every single test before you write any production code. Contrarily, we’ll basically keep the unit tests one small step ahead of the production code, and develop both at the same time. By doing this cycle of writing a test, production-coding until the test passes, and then repeating, we constantly keep the code in an unbroken state.

First, we’ll write a unit test for the “happy case,” where the the correct answer to the chosen question is given. We will create a QuestionAndAnswerValidator object to expose a simple interface for clients to get the possible questions, as well as validate potential answers to those questions. Our unit tests will reside in the QuestionAndAnswerValidatorTestCase class. Here is our first “happy case” unit test:

public function testValidateCorrectAnswerToQuestion() 
{
 $question_and_answer_validator = $this->getQuestionAndAnswerValidatorToTest();
 $question_key = QuestionAndAnswerKeys::favorite_aircraft_make_key;
 $answer_to_validate = 'Cessna';

 $error_message_or_success = $question_and_answer_validator->validateAnswerToQuestion($question_key, $answer_to_validate);
 $this->assertTrue($error_message_or_success, "The validate method should return true for the correct answer.");
}

If we run the unit tests now, we would get an error about the QuestionAndAnswerValidator::validateAnswerToQuestion method not existing. We will implement it below, and simply return “true” to denote that the answer is correct.

public function validateAnswerToQuestion($question_key, $answer_to_validate)
{
  return true;
}

Even though this isn’t correct production code yet, now we have a passing unit test to start off validating the “happy case”. As we continue refactoring, this will make sure we don’t have a regression error for the happy case.

Continuing Responsibility Transfer – The First Unhappy Test Case

To continue moving question/answer validation responsibility over to our new class, we will again start with a unit test:

public function testValidateNoAnswerToQuestionGiven()
{
  $question_and_answer_validator = $this->getQuestionAndAnswerValidatorToTest();
  $question_key = QuestionAndAnswerKeys::favorite_number_key;
  $answer_to_validate = '';
  $expected_error_message = "You didn't guess anything!";

  $error_message_or_success = $question_and_answer_validator->validateAnswerToQuestion($question_key, $answer_to_validate);
  $this->assertTrue($error_message_or_success === $expected_error_message, "The validate method should return an error message for incorrect input.");
}

The above unit test simply checks for the correct error message when an empty string is passed to the validator object.

This test fails, until we implement the code that will make it pass. The following implementation coincides with validation responsibility #1 (under Planning Our Extraction / Original Responsibilities section of part 1) of the original GuessForm::validateForm method at the start:

public function validateAnswerToQuestion($question_key, $answer_to_validate)
{
  $error_message = "";

  // 1
  if (empty($answer_to_validate))
    $error_message = "You didn't guess anything!";

  if (empty($error_message))
    return true;
  else
    return $error_message;
}

Hopefully this process makes sense. Again, we are creating one test at a time, and just implementing enough production code to make the test that we just created pass. We do no more production code than that, until we create another failing unit test. Also note that we haven’t touched the GuessForm object yet. We’ll refactor that momentarily. (This commit shows the code state and passing unit tests just after the first unhappy use case.)

Next Unhappy Test Case – Creating QuestionAndAnswer Objects

We have two more unhappy test cases to cover on the QuestionAndAnswerValidator object. We’ll work through just one at a time, and so here is the unit test for the next unhappy test case:

public function testValidateIncorrectAnswerToQuestion()
{
 $question_and_answer_validator = $this->getQuestionAndAnswerValidatorToTest();
 $question_key = QuestionAndAnswerKeys::favorite_number_key;
 $answer_to_validate = '-12345';
 $expected_error_message = "You entered an incorrect answer to the question you were trying to guess.";

 $error_message_or_success = $question_and_answer_validator->validateAnswerToQuestion($question_key, $answer_to_validate);
 $this->assertTrue($error_message_or_success === $expected_error_message, "The validate method should return an error message for incorrect input.");
}

Of course, we run this and it fails, until we implement just enough production code to make it pass. The above test case covers validation responsibility #5 of the original GuessForm::validateForm method.

Before we validate incorrect answers, though, let’s discuss how to come up with a way to create QuestionAndAnswer objects that tie the questions and answers together. We’ll implement a very rudimentary version of the factory method pattern. In our case, we aren’t making the factory a separate object, but simply a method on the QuestionAndAnswerValidator. If constructing the QuestionAndAnswer objects were any more involved (such as if the data required to build the QuestionAndAnswer instances were stored in a database, or if we wanted to have different subclasses of the QuestionAndAnswer class), then moving this creation logic to it’s own class might be more beneficial.

The factory method just discussed is called QuestionAndAnswerValidatorTestCase::getQuestionAndAnswerForKey, and while it’s implementation isn’t shown in this article, it’s usage is displayed in the updated QuestionAndAnswerValidator::validateAnswerToQuestion below. Also, we don’t show the QuestionAndAnswer::validateAnswer method, but it simply returns true or false based on the passed-in answer being equal to the encapsulated, correct answer to the question. (Here is the code state for this specific point.) With this new validateAnswerToQuestion implementation, our unit tests, including our most recent failing test, will now pass:

public function validateAnswerToQuestion($question_key, $answer_to_validate)
{
  $error_message = "";
  $question_and_answer_object = $this->getQuestionAndAnswerForKey($question_key);

  // 1
  if (empty($answer_to_validate))
  {
    $error_message = "You didn't guess anything!";
  }
  // 5
  else if (!$question_and_answer_object->validateAnswer($answer_to_validate))
  {
    $error_message = "You entered an incorrect answer to the question you were trying to guess.";
  }

  if (empty($error_message))
    return true;
  else
    return $error_message;
}

I believe it’s worth pointing out the purpose of our earlier tests. When we first moved the happy test case over to the QuestionAndAnswerValidator class, the only code in the validateAnswerToQuestion method was just “return true;” — only enough to make that first happy case unit test pass. The validateAnswerToQuestion method has obviously changed much since then, but that first happy case unit test has still served it’s purpose — ensuring a correct answer produces a true return value. Basically, previous unit tests help ensure that we aren’t actively breaking code as we go about refactoring or enhancing our code base.

Moving the Last Unhappy Test Case

Here is the last unhappy test case we care about. Part of the original functionality is that we get different error messages generated when the answer given is not correct for the selected question, but that it is a correct answer for a different question in the collection that isn’t selected. (ie. in the code below, the “Cessna” is not the correct answer to the “what’s my favorite number?” question, but it is the answer to the “what’s my favorite airplane?” question. If “what’s my favorite number?” is the selected question, and “Cessna” is given as the answer, we don’t just want to tell the user that the answer is wrong, but rather tell them that it’s wrong for the “what’s my favorite number?” question, but correct for the“what’s my favorite airplane?” question). Here’s our unit test implementation:

public function testValidateCorrectAnswerGivenForNotCurrentlySelectedQuestion()
{
  $question_and_answer_validator = $this->getQuestionAndAnswerValidatorToTest();
  $question_key = QuestionAndAnswerKeys::favorite_number_key;
  $answer_to_validate = 'Cessna';
  $expected_error_message = "You entered an incorrect answer to the question you were trying to guess, "
    . "but the answer was ironically correct for a different question. Change the question and then click 'guess again'!";

  $error_message_or_success = $question_and_answer_validator->validateAnswerToQuestion($question_key, $answer_to_validate);
  $this->assertTrue($error_message_or_success === $expected_error_message, "The validate method should return an error message for incorrect input.");
}

The test fails, to make it pass, we’ll first edit the QuestionAndAnswerValidator::validateAnswerToQuestion method. The next validation corresponds to validation responsibility #4 of the original GuessForm:validateForm method:

public function validateAnswerToQuestion($question_key, $answer_to_validate)
{
  $error_message = "";
  $question_and_answer_object = $this->getQuestionAndAnswerForKey($question_key);

  // 1
  if (empty($answer_to_validate))
  {
    $error_message = "You didn't guess anything!";
  }
  else 
  {
    // 5
    if (!$question_and_answer_object->validateAnswer($answer_to_validate))
    {
      $error_message = "You entered an incorrect answer to the question you were trying to guess.";
    }

    // 4
    if (!empty($error_message)
        && $this->checkAnswerIsCorrectForOtherThanSpecifiedQuestion($question_key, $answer_to_validate))
    {
      $error_message = "You entered an incorrect answer to the question you were trying to guess, "
        . "but the answer was ironically correct for a different question. Change the question and then click 'guess again'!";
    }
  }

  if (empty($error_message))
    return true;
  else
    return $error_message;
}

Notice that in the above code, there is a call to a new method, QuestionAndAnswerValidator::checkAnswerIsCorrectForOtherThanSpecifiedQuestion. I intend to write about the significance of breaking out that code into a separate method in a later article. At any rate, this new method contains the remaining logic that was in the original GuessForm::validateForm for validation responsibility #4, to catch the situation where the given answer satisfies a question that wasn’t the selected question:

protected function checkAnswerIsCorrectForOtherThanSpecifiedQuestion($question_key, $answer_to_validate)
{
  $answer_is_correct_for_other_than_specified_question = false;
  $all_question_keys_except_parameter_question_key = $this->getAllQuestionKeysExcept($question_key);

  foreach ($all_question_keys_except_parameter_question_key as $current_question_key)
  {
    $question_and_answer_object = $this->getQuestionAndAnswerForKey($current_question_key);

    if ($question_and_answer_object->validateAnswer($answer_to_validate))
    {
      $answer_is_correct_for_other_than_specified_question = true;
      break;
    }
  }

  return $answer_is_correct_for_other_than_specified_question;
}

After implementing the last unhappy test case and the corresponding production code to make all of the tests pass, this is the state of our code base.

Part 2 Conclusion

Up to this point, we’ve basically finished all of the production code and corresponding unit tests for the QuestionAndAnswerValidator class. In the next article in this series, we will complete the process of making the GuessForm::validateForm method use the new QuestionAndAnswerValidator, covered first by unit tests. We will leverage Dependency Injection (DI) in order to make the GuessForm aware of the QuestionAndAnswerValidator class.

Leave a Reply