views:

269

answers:

3

I have a lump of code that looks a bit like this:

If mode = DiaryMode.Admin Then
    modeObject = DBNull.Value
ElseIf mode = DiaryMode.Academy Then
    modeObject = ApplicationSettings.Academy
ElseIf mode = DiaryMode.Scouting Then
    modeObject = ApplicationSettings.Scouting
Else
    Throw New NotSupportedException()
End If

The idea of the check is to prep some values for passing into a database call.

There are two questions, is the Else worth the effort? The intention is to prevent future extensions of the enum causing the code to return squify results.

If the code is valid, I'd like to be able to unit test the behaviour, if it's worth having it's worth testing. How might I go about doing that?

A: 

You'd rather refactor this using Replace Conditional with Polymorphism.

Writing a test case for these lines will prove that the "if...then...else" statement is returning the correct modeObject, but I wouldn't spend much time on it. It may not be completely pointless, though: if you plan to add new modes the test will ensure that they would be handled as expected, expecially if there's going to be some cut & paste involved (it's not unusual to paste the code and fail to modify it accordingly).

That's what I would do: I'd write the test case(s), then refactor to have a hierarchy of classes handling the modes, and then re-run the tests to make sure I didn't break anything. Sounds like a plan?

Manrico Corazzi
A: 

Adding the Else seems reasonable enough. If you use the built-in Visual Studio unit testing, there is actually an attribute to indicate that you expect a test to throw an exception for success: ExpectedExceptionAttribute.

jerryjvl
A: 

If your business requirements state that no other DiaryMode is acceptable, then an else statement is a perfectly fine use to prevent future extension on your DiaryMode enum.

As for testing, that gets a little bit trickier. I would test all valid states (Admin, Academy, and Scouting) for sure. However, you can't really set the mode to an enum value that doesn't exist, and that would be the only way to throw the NotsupportedException. I might look into trying to write a test that would verify that the enum only carries the enumerations your expecting.

You could do that by doing something like this:

Enum.GetNames(typeof(DiaryMode))

and then verifying each name.

To sum up, you'd have 4 tests from what I've described.

  • Admin test
  • Academy test
  • Scouting test
  • Test that checks all the names in the DiaryMode enum
Joseph
This seems like the most appropriate answer in my scenario, thanks.
ilivewithian
Glad I could help!
Joseph