CodeBetter.Com
CodeBetter.Com
RSS 2.0 via Feedburner
           Do you Twitter? Follow us @CodeBetter

Patrick Smacchia [MVP C#]


A simple trick to code better and to increase testability

The rule consists in minimizing what is called the Nesting Depth. The Nesting Depth is a metric defined on methods that is relative to the maximum depth of the more nested scope in a method body. For example the following method has a Nesting Depth of 3.

 

public static void Method(List<string> list) {

   for (int i = 0; i < 10; i++) {

      if (i > 5) {

         foreach (string s in list) {

            Console.WriteLine(s);

         }

      }

   }

}

 

Nesting Depth is not so obvious when it comes to complex if statement. For example the following method has a Nesting Depth of 3 even though it has only one if statement.

 

public static void Method(string s) {

   if (s != null && s.Length > 0 && s.Contains("a")) {

      Console.WriteLine(s);

   }

}

 

Obviously you want to keep the Nesting Depth of your methods low. But it is not only about readability but it is also for testability. Here is a common situation where choosing for lower Nesting Depth enhance the testability of the code.

 

public static void Method(List<string> list) {

   foreach (string s in list) {

      if (s != null &&

          s.Length > 0 &&

          s.Contains("a") /*...*/ ) {

         Console.WriteLine(s);

      }

   }

}

public static void Method(List<string> list) {

   foreach (string s in list) {

      if (s == null) { continue; }

      if (s.Length == 0) { continue; }

      if (!s.Contains("a")) { continue; }

      // ...

      Console.WriteLine(s);

   }

}

 

The second code is more readable because what does matter, the WriteLine() call, has a Nesting Depth of 1 instead of 4. The code is more testable also because now, if we don’t write some unit tests to cover the 3 cases where the Writeline() is not invoked, we won’t have 100% coverage.

 

The same trick applies to methods that return something, for example:

 

public static int Method(string s) {

   if (s == null) { return 0; }

   if (s.Length == 0) { return 0; }

   if (!s.Contains("a")) { return 0; }

   // ...

   return s.GetHashCode();

}

 

…is better than…

 

public static int Method(string s) {

   if ( s == null ||

        s.Length == 0 ||

        !s.Contains("a") /*...*/ ) {

      return 0;

   }

   return s.GetHashCode();

}

 

…because it is both more readable and more testable.

 

Hopefully the new NDepend version 2.7 has support for Nesting Depth. The following CQL rule will ensure that you don't get too high in Nesting Depth:

 

WARN IF Count > 0 IN SELECT METHODS WHERE ILNestingDepth > 4

 

The implementation infers Nesting Depth from the IL code. It means that it works no matter the language you choose and that Nesting Depth values are comparable amongst languages. It means also that you have another way to measure the quality of third parties code you rely on, because you don’t need the source files to measure it, only the assemblies are needed (and it doesn’t matter if they are obfuscated or not). Who cares that in mscorlib the method System.Text.UnicodeEncoding.GetBytes(Char*,Int32,Byte*,Int32,EncoderNLS) has a Nesting Depth value of 16!

 

The only drawback of inferring the Nesting Depth from IL is that sometime values are a bit higher than expected because of C# and VB.NET compiler optimizations, especially on switch with many case conditions. For example, when a switch on a string has more than 6 cases, the C# compiler estimates that it is worth creating a generic Dictionary<string,int> and to do the switch on the hash value of the string. Have a look at the IL of the following method:

 

public static int Method(string s) {

   switch (s) {

      case "1":

         return 1;

      case "2":

         return 2;

      case "3":

         return 3;

      case "4":

         return 4;

      case "5":

         return 5;

      case "6":

         return 6;

      case "7":

         return 7;

   }

   return 0;

}



Comments

Neil Mosafi said:

Hi, good article, and I do completely agree with your points and try to do this where ever I can.  

Maybe I am misunderstanding, but I am not sure how this increases testability.

I think what you saying is that you create more separate lines of code, therefore code coverage tools are more likely to pick up that you haven't tested all conditions.  But you could still test all the conditions even with a high level of nesting.  So why is it more inherently testable?

# March 7, 2008 7:35 AM

Derik Whittaker said:

Patrick,

I think what you are trying to say is that your code should follow the KISS principle.  Doing so makes everything easier.

# March 7, 2008 7:41 AM

Mark Grant said:

Hi Patrick,

I think you have made two points here rolled into one. One I agree with the other I do not.

The first is that nesting depth is a sign of complexity and as Derik said, KISS should apply.

The second is that nesting occurs in algebraic expressions, algebraic expressions are less readable/testable than if statements and therefore we should use if statements. On this I disagree on a number of counts.(expecting to be flamed)

Taking your example:

public static int Method(string s) {

  if (s == null) { return 0; }

  if (s.Length == 0) { return 0; }

  if (!s.Contains("a")) { return 0; }

  // ...

  return s.GetHashCode();

}

This can be written as an expression and simplified by extracting functions,

public static int Method(string s) {

  return s != null && s.Length != 0 && !s.Contains("a") ? s.GetHashCode() : 0;

}

This may upset some people who can't abide the tertiary operator, but hey, can't please everyone. And if this is too complex then we can extract the test like this:

public static int Method(string s) {

  return IsValid(s) ? s.GetHashCode() : 0;

}

public static bool IsValid(string s) {

  return s != null && s.Length != 0 && !s.Contains("a") ;

}

Why do I prefer this?

1. its easy to read (this of course depends on what you are used to)

2. I can shrink my code dramatically

3. refactoring becomes easier, because it is declarative not imperative (with small functions this may not make a huge difference)

# March 7, 2008 9:08 AM

Daily Bits - March 7, 2008 | Alvin Ashcraft's Daily Geek Bits said:

Pingback from  Daily Bits - March 7, 2008 | Alvin Ashcraft's Daily Geek Bits

# March 7, 2008 9:25 AM

Patrick Smacchia said:

Yes, follow the KISS principle. But as other comment suggest not everybody agree on what is Simple (first S in KISS). That is why I expose concrete example.

Mark, shrinking code shouldn't be the goal. Following this principle to the extreme you would name your variables 'a' 'b' and 'c'.

>I think what you saying is that you create more separate lines of code, therefore code coverage tools are more likely to pick up that you haven't tested all conditions.  

Exactly. Make sure that each 'atomic' test success and each test failure result in at least one line of code (sequence point) covered. This way you can make sure with test coverage that your set of unit test is complete.

In this example:

public static bool IsValid(string s) {

 return s != null && s.Length != 0 && !s.Contains("a") ;

}

I can easily have 100% coverage by testing:

Assert.IsTrue(IsValid("b"));

Assert.IsFalse(IsValid(null));   // <-- actually you don't even need this one to get 100% coverage

And as you can see I don't test the case where (s.Length == 0).

It is because test coverage tool (MSTest, NCover...) are based on sequence points and shrinking code with ternary operator and complex if statement leads to less sequence point.

# March 7, 2008 9:54 AM

DotNetKicks.com said:

You've been kicked (a good thing) - Trackback from DotNetKicks.com

# March 7, 2008 10:51 AM

Peter Ritchie's MVP Blog said:

Before the modern high-level languages Edsger Dijkstra came up with &quot;Structured Programming&quot;

# March 7, 2008 10:52 AM

Jeremy Gray said:

As much as I like your product and your series of posts in general, I think you are largely just hiding the pea in this case, Patrick.

Good test coverage is based not on lines covered but rather on _paths_ covered. Breaking out the code the way you have in your examples only has a panacea effect of making you _think_ you've covered your code better. In reality, not only have you not covered the code any better you have in each case made the code longer, generally less readable, and have made it contain more redundancies. Each of these would run counter to overall maintainability even _if_ you increased testability, which you haven't.

# March 7, 2008 2:37 PM

Patrick Smacchia said:

Jeremy, it is interesting we disagree on this.

>Good test coverage is based not on lines covered but rather on _paths_ covered.

100% code covered is the first goal and then come path coverage.

But still, taken account on how code coverage tool works (all based on PDB sequence points) one has to decompose its if statement, else one never knows what was tested or not simply because the code coverage technology is not able to do the decomposition by itself.

It is a pragmatic attitude because things won't likely change for a long time in the field of sequence points and PDBs, hence in the field of code coverage in .NET.

# March 7, 2008 3:32 PM

Christopher Steen said:

Link Listing - March 8, 2008

# March 8, 2008 11:11 AM

Christopher Steen said:

AJAX ASP.NET AJAX In-Depth: Application Events [Via: swalther ] WPF WPF Wiimote Library (Now With Project...

# March 8, 2008 11:12 AM

Reflective Perspective - Chris Alcock » The Morning Brew #49 said:

Pingback from  Reflective Perspective - Chris Alcock  &raquo; The Morning Brew #49

# March 10, 2008 4:21 AM

Rory Primrose said:

I read Patrick Smacchia's post this morning about a simple trick to code better and to increase testability

# June 1, 2008 11:14 PM

Cialis best price buy online. said:

Cialis online. Cialis best price buy online.

# July 22, 2008 1:19 AM

Leave a Comment

(required)  
(optional)
(required)  

Enter the numbers above:
Add

About Patrick Smacchia

Patrick Smacchia is a Visual C# MVP involved in software development for over 15 years. After graduating in mathematics and computer science, he has worked on software in a variety of fields including stock exchange, airline ticket reservation system as well as a satellite base station at Alcatel. He's currently a software consultant and trainer on .NET technologies as well as the lead developer of the tool NDepend which provides numerous metrics and caveats on any compiled .NET application. He is the author of Practical .NET2 and C#2, a .NET book conceived from real world experience with 647 compilable code listings. Check out Devlicio.us!