Geek Answers Handbook

Best practice for programming and testing modules

I came across a script that I am not too sure how to approach.

We are trying to get unit test code coverage of up to 100%.

I am trying to use the TDD approach to development, write tests, do it, write a failed test, add more code to make it pass, etc.

In doing so, 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 extension was expanded:

 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 ask is:
Now that the factory is doing an enumeration, do I remove the default exception to cover the code, or keep the exception there by default if someone adds a new type to the enumeration and forgets to change the factory?

The second question I ask is:
If I decided to remove the exception and use the UnpaidBreakQuarter type by default - does it make sense to return IQuarter to UnpaidBreakQuarter by default, or it will raise the question of why UnpaidBreakQuarter is used by default, is there a business rule for this? "

Hello,

James

+4
source share
4 answers

You can still get 100% code coverage. Consider the following line, which simply compiles:

 QuarterFactory.CreateQuarter((IntervalType)0);

Thus, this answers the second question. You can get really confusing results if this returns UnpaidBreakQuarter .

Shortly speaking:

  • Keep the default value, unit test, to throw an exception if the transfer is unexpected (for example, the line above)
  • Do not do this. If someone did not indicate that they want it, they did not expect him to be returned.

You can even get an exception by creating this:

 QuarterFactory.CreateQuarter(0);
+5
source

I think you should keep the default block, this is a good practice, especially for the case that you already mentioned, that is, if someone changes the code in the future by adding a new IntervalType.

The answer to the second question arises as follows :) By the way, using the default value for the specified value, because "I know that the only value left as a result is really terrible, the default value was intended specifically for exceptions or unexpected cases, or at most for the most common (i.e. the first is not, the second is not, is normal, so ANY other case should be this value).

+2
source

I think that you should default by covering your 100% code if there is no business rule for this and it has been documented. If you decide to save it when someone adds another value to the enumeration, the exception will be selected as a “reminder” to add a case to the switch statement, which I think is good.

+1
source

Do you know that this kind of switch / enum combo is usually a smell of code? http://c2.com/cgi/wiki?SwitchStatementsSmell

You could use refactoring to use polymorphism: this would provide 100% coverage, make it easy to add new cases, and not require a default value because the compiler would prevent it.

The two most common refactorings in this case are “Replace type code with class” and more specific “Replace type code with strategy”. Of course, it is also worth mentioning "Replace conditional with polymorphism." http://refactoring.com/catalog/replaceConditionalWithPolymorphism.html

0
source

More articles:


All Articles