Telerik blogs

If you’ve never done Test Driven Development or aren’t even sure what this "crazy TDD stuff” is all about than this is the series for you. Over the next 30 days this series of posts take you from “I can spell TDD” to being able to consider yourself a “functional” TDD developer. Of course TDD is a very deep topic and truly mastering it will take quite a bit of time, but the rewards are well worth it. Along the way I’ll be showing you how tools like JustCode and JustMock can help you in your practice of TDD.

At this point we’ve gotten a pretty good amount of code written between our tests and our business code. As time goes on in any software development project you’ll no doubt find inefficiencies in your code that you would like to remove. Other times you’ll receive new requirements that are going to necessitate large scale changes in your existing code. And you will still occasionally find code that you’ve written in the past that will make you ask “What was I thinking when I did that?!” When these situations arrive, it’s time to look at refactoring your code.

Previous Posts in this Series Day Eight – Dealing with Defects

Time For a Code Review

In the last installment of this series we dealt with a defect in our code. This left us with business code that looked like this:
 
 1:  public int FindNumberOfOccurences(string sentenceToScan, string characterToScanFor)
 2:         {
 3:  try
 4:             {
 5:                 var stringToCheckAsCharacterArray = sentenceToScan.ToCharArray();
 6:                 var characterToCheckFor = Char.Parse(characterToScanFor);
 7:  
 8:                 var numberOfOccurenes = 0;
 9:  
 10:  for (var charIdx = 0; charIdx < stringToCheckAsCharacterArray.GetUpperBound(0); charIdx++)
 11:                 {
 12:  if (stringToCheckAsCharacterArray[charIdx] == characterToCheckFor)
 13:                     {
 14:                         numberOfOccurenes++;
 15:                     }
 16:                 }
 17:  
 18:  return numberOfOccurenes;
 19:             }
 20:  catch
 21:             {
 22:  throw new ArgumentException();
 23:             }
 24:         }
(get sample code)

The defect we responded to last time was:
A user was able to pass in a two character string for the second argument to FindNumberOfOccurences and an exception of type FormatException was no thrown. The expected behavior is for the method to throw an exception of type ArgumentException.

My solution to this was to simply wrap the entire method in a try/catch block and throw an ArgumentException exception if an error occurred. It satisfied the defect, but in terms of being “good code” it comes up a bit short. It does provide us with a good opportunity to do some refactoring.

Martin Fowler has defined refactoring as “… a controlled technique for improving the design of an existing code base.” Essentially refactoring is about finding ways to make our code better. “Better” can mean many things. Sometimes it means changing code to increase readability. Sometimes it means removing instances of repeated logic or code. Sometimes it means separating large methods or classes to make them more manageable. There are many reasons to refactor code. In a future post I’ll cover some common “code smells” and how to deal with them.

TDD easily enables refactoring. Without unit tests I would have concerns about altering working code. Short of an extensive QA process or releasing the application to the users and waiting to see if another defect manifests, I really have no way to verify that my application still meets the business requirements, as embodied by the unit tests. Because I have a working set of unit tests I can feel free to refactor my code and so long as my tests pass I know that no matter what I’ve done to my code, it still meets the requirements.

When refactoring you want to make small changes and then run the unit tests to verify that what you’ve done has not damaged your application. Small changes ensure that if your change doesn’t work (meaning that your tests fail) it’s easy to back out.

In the current implementation of the FindNumberOfOccurences method, the technique to fix the defect was probably not the best way to do it. What we probably really want to do is have an active validation rule that checks the length of the characterToScanFor parameter and throws the exception if the length is not one. I will change my implementation of FindNumberOfOccurences to use this approach instead of just capturing an exception and throwing a new one:

 1:  public int FindNumberOfOccurences(string sentenceToScan, string characterToScanFor)
 2:         {
 3:  if (characterToScanFor.Length != 1)
 4:             {
 5:  throw new ArgumentException();
 6:             }
 7:             var stringToCheckAsCharacterArray = sentenceToScan.ToCharArray();
 8:             var characterToCheckFor = Char.Parse(characterToScanFor);
 9:  
 10:             var numberOfOccurenes = 0;
 11:  
 12:  for (var charIdx = 0; charIdx < stringToCheckAsCharacterArray.GetUpperBound(0); charIdx++)
 13:             {
 14:  if (stringToCheckAsCharacterArray[charIdx] == characterToCheckFor)
 15:                 {
 16:                     numberOfOccurenes++;
 17:                 }
 18:             }
 19:  
 20:  return numberOfOccurenes;
 21:         }

(get sample code)

Instead of wrapping the whole method in a try/catch block I am checking the length of the characterToScanFor parameter. If the length is not one I am throwing an AgurmentException exception. This is a much better approach and I’m actively checking a validation rule as opposed to just throwing an error to satisfy a requirement (“Works by Coincidence” is a code smell that will be covered in a future post). This also prevents me from swallowing a defect that is not related to my validation rule. All in all this is a much better approach. But does it work? The only way to know is to run the tests (Figure 1):

image

Figure 1 – Our test still pass

Again, since the tests still pass we know that no matter what changes we’ve made the business logic still works, in that it meets the needs of the requirements that have driven our tests. Other defects and new requirements may arise in the future, but as of now our code meets our business needs.

Refactoring is an important step in TDD. You will often hear the phrase “Red, Green, Refactor.” The “Red” refers to the idea that we write a test and see it fail first. Once the code has been written to make the test pass, we have reached the “Green” part of our workflow; our code works and meets the given need of the application as defined by the unit test. Refactoring is the final step. Periodically refining our code make it more readable, more optimized, easier to work with and more flexible overall.

Summary

In this post we discussed the basics of refactoring and why it’s important. This information will factor heavily into future posts about mocking, additional feature of NUnit and common refactoring patterns. In the next post I’ll show you how the practice of refactoring extends to our unit tests themselves.

Continue the TDD journey:

JustCode download banner image

JustMock banner


About the Author

James Bender

is a Developer and has been involved in software development and architecture for almost 20 years. He has built everything from small, single-user applications to Enterprise-scale, multi-user systems. His specialties are .NET development and architecture, TDD, Web Development, cloud computing, and agile development methodologies. James is a Microsoft MVP and the author of two books; "Professional Test Driven Development with C#" which was released in May of 2011 and "Windows 8 Apps with HTML5 and JavaScript" which will be available soon. James has a blog at JamesCBender.com and his Twitter ID is @JamesBender. Google Profile

Comments

Comments are disabled in preview mode.