Thursday, May 22, 2008

Unit Testing Switch Statements

A standard practice of mine when using switch statements with enum values is to throw a NotImplementedException on the default case.  This is so that when I add a new enum value, the function will bomb if I didn't account for that value in one of the switch statements.

Take the following example.  I have an enum called MyValues.  I've recently added Value3.  All the unit tests that I've written for my method MyClass.DoSomething pass, so I'm done... Right?  Wrong!  My switch statement doesn't explicitly use the new value and it should.  The unit test technically passes because no exception is thrown, but I wanted it to do something with that value, but instead it did nothing.

public enum MyValues
{
	Value1,
	Value2,
	Value3 //	Added to support feature #123
}

public static class MyClass
{
	public static void DoSomething(MyValues myValue)
	{
		switch (myValue)
		{
			case MyValues.Value1:
				System.Console.WriteLine("Did first action.");
				break;
			case MyValues.Value2:
				System.Console.WriteLine("Did second action.");
				break;
		}
	}
}

[NUnit.Framework.TestFixture]
public class MyClass_Tester
{
	[NUnit.Framework.Test]
	public void DoSomething_Test01()
	{
		MyClass.DoSomething(MyValues.Value1);
	}

	[NUnit.Framework.Test]
	public void DoSomething_Test02()
	{
		MyClass.DoSomething(MyValues.Value2);
	}

	[NUnit.Framework.Test]
	public void DoSomething_Test03()
	{
		MyClass.DoSomething(MyValues.Value3);
	}
}

Ok, let's go ahead and change that switch statement to now throw an exception on the default case.

switch (myValue)
{
	case MyValues.Value1:
		System.Console.WriteLine("Did first action.");
		break;
	case MyValues.Value2:
		System.Console.WriteLine("Did second action.");
		break;
	default:
		throw new System.NotImplementedException(
			string.Format("The switch statement does not have an implementation for the MyValues value {0}.", myValue));
}

Now my unit test DoSomething_Test03() fails and I realize that I haven't accounted for MyValues.Value3 in MyClass.DoSomething.

switch (myValue)
{
	case MyValues.Value1:
		System.Console.WriteLine("Did first action.");
		break;
	case MyValues.Value2:
		System.Console.WriteLine("Did second action.");
		break;
	case MyValues.Value3:
		System.Console.WriteLine("Did third action.");
		break;
	default:
		throw new System.NotImplementedException(
			string.Format("The switch statement does not have an implementation for the MyValues value {0}.", myValue));
}

But I still haven't figured out how to test the default case in unit tests.  This means I do not have 100% code coverage.  It's a moot point as the default case is a simple throw statement, but I really want to achieve 100% code coverage on this class.  For this, I'm modifying the DoSomething method to employ a Design-By-Contract class (find the code here) to check the enum values  So now my code looks like this.

public static class MyClass
{
	public static void DoSomething(MyValues myValue)
	{
		Check.Require(myValue == MyValues.Value1 || 
			myValue == MyValues.Value2 || 
			myValue == MyValues.Value3);

		switch (myValue)
		{
			case MyValues.Value1:
				System.Console.WriteLine("Did first action.");
				break;
			case MyValues.Value2:
				System.Console.WriteLine("Did second action.");
				break;
			case MyValues.Value3:
				System.Console.WriteLine("Did third action.");
				break;
		}
	}
}

Now I can achieve 100% code coverage.  And you'll notice, I no longer need the default case since I've already listed the enum values that I support in the function at the very top of the function.  If I wrote my unit test better to automatically just test every value of that enum without me having to explicitly define a test for each enum value, then I could switch that Check.Require to Check.Invariant and then that check won't even occur during production execution (as long as you don't have the DBC_CHECK_INVARIANT conditional compilation symbol defined in your Release configuration).  I can do this because I know that the unit tests will catch any enum values I don't support.

Submit this story to DotNetKicks

1 comments:

SuperJason said...

That does make more sense as a precondition. You're defining a contract that the caller needs to comply with.