Programming & Unit Testing Best Practice

Question

I've come across a scenario that I'm not too sure how to approach.

We're trying to get our unit tests code coverage to 100%.

I've been trying to take a TDD approach to development, writing tests, making it pass, writing a failing test, adding more code to make it pass, etc.

While doing this I found that I wrote a class like this:

public enum IntervalType : int
{
    Shift = 1,
    PaidBreak = 2,
    UnpaidBreak = 3
}


public static class QuarterFactory
{
   public static IQuarter CreateQuarter(IntervalType intervalType)
   {
      switch (intervalType)
      {
           case IntervalType.Shift:
                return new ShiftQuarter();
           default:
                throw new ArgumentException(String.Format("Unable to create a Quarter based on an IntervalType of: {0}", intervalType));
       }
    }
 }

As coding progressed the factory expanded to this:

public static class QuarterFactory
{
    public static IQuarter CreateQuarter(IntervalType intervalType)
    {
        switch (intervalType)
        {
            case IntervalType.Shift:
                return new ShiftQuarter();
            case IntervalType.PaidBreak:
                return new PaidBreakQuarter();
            case IntervalType.UnpaidBreak:
                return new UnpaidBreakQuarter();
            default:
                throw new ArgumentException(String.Format("Unable to create a Quarter based on an IntervalType of: {0}", intervalType));
        }
    }
}

The first question I'm asking is:
Now that the factory fulfills the enum, do I remove the default exception for the sake of code coverage, or do I keep the exception in there as the default in the event that someone adds a new type to the enum and forgets to modify the factory?

The second question I'm asking is:
If I decided to remove the exception and default the type to be UnpaidBreakQuarter - does it make sense to default the IQuarter returned to UnpaidBreakQuarter, or would this raise the question of "why is the default the UnpaidBreakQuarter, is there a business rule for this?".

Regards,

James

Answer 1

You can still get 100% code coverage. Consider the following line that compiles just fine:

QuarterFactory.CreateQuarter((IntervalType)0);

Thus, this answers the second question as well. You can get really confusing results if that would return an UnpaidBreakQuarter.

So in short:

1. Keep the default, unit test that it throws exception if passing something unexpected (like the line above)
2. Don't do it. If someone did not specify that they wanted this, they wouldn't expect it to be returned

You can even get the exception to be raised by doing this:

QuarterFactory.CreateQuarter(0);

Answer 2

I think you should keep the default block, it's a good practice, especially for the case you already mentioned, that is if someone would modify the code in the future adding a new IntervalType.

The answer to the second question comes consequently :) Btw, using the default for a specified value because "I know that the only value left is that" is really horrible, the default value was intended exactly for exceptions or unexpected cases, or at most for the most general ones (i.e: the first not, the second not, ok, so ANY other case should be this value).