Home > .NET, Testing > Enums and inheritance in .Net

Enums and inheritance in .Net

In one of my current projects I had the following code (I simplified the code a bit):

public string ConnectionString
{
	get
	{
		switch(this.Importer)
		{
			case Importer.SqlServer:
				return "Server=localhost;Database=Northwind";
			case Importer.SqlServerOleDb:
				return"Provider=SQLOLEDB;Data Source=localhost;Initial Catalog=Northwind";
			default:
				throw new NotSupportedException(
				string.Format("Importer {0} is not supported yet.", this.Importer));
		}
	}
}

After running the code coverage tool (dotCover from JetBrains) I received the following picture:

CodeCoverage

First idea

So, my code was clear and understandable, but obviously not fully tested (green: covered by tests, red: not covered). I asked me the question, how could I test the rest of the method.

My idea was to extend the enum:

[TestFixture]
public class ConfigurationTest
{
        #region Test methods
        #endregion

        private enum TestImporter : Importer
        {
            Test
        }
}

But the compiler and the ReSharper weren’t happy with my idea: “Type byte, sbyte, short, ushort, int, uint, long, or ulong expected”. I was surprised at this error: Why it isn’t possible to make a subclass of an enum?

I found the explanation first on stackoverflow and then in the specification of the Common Language Infrastructure (CLI, section 8.5.2):

It shall derive from System.Enum

An enum in .Net has to derive directly from enum, it isn’t possible to inherit from another enum. So, the enums in .Net don’t follow the open/closed principle (OCP). With the OCP you don’t modify a class when you want to extend it, you inherit from it and do the extensions in the subclass:

Software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.

Second idea

My code wasn’t tested and is it really not possible to test it? Well, it is possible, but the code isn’t very nice (hard cast):

 [Test]
[ExpectedException(typeof(NotSupportedException))]
public void TestConnectionString_With_Invalid_Enum()
{
	// Arrange
	this.Configuration.Importer = (Importer) 100;

	// Act
	string strConnectionstring = this.Configuration.ConnectionString;
}

Conclusion

I my current decision is to leave the code as it is. The reasons were the following:

  • Readability is very important
  • You have to use default or a return value, otherwise the code will not compile
  • I want that my tests will fail when I extend the enum and test the new value. So it is easier to find the places where I have to adjust something (but it smells like shotgun surgery…)
  • The code is testable, but you have to create an invalid enum value by hard casting an integer (and that is ugly)

I’m not really convinced with my current decision, but I could live with it. How would you design the code? Are there any better ways to test the code?

If you like this, follow me on twitter…

Share
Categories: .NET, Testing Tags:
  1. February 9th, 2011 at 17:07 | #1

    Simple solution: do not use enums to define behavior, use constants/reference codes. Enums are bad considering OCP. How should I add a new Importer without changing the enum values?

    Cheers Urs

  2. February 9th, 2011 at 17:11 | #2

    @Urs Enzler
    Thanks for the comment, Urs.

    Your completely right. I would also now prefer classes with constants or static readonlys. In this way I could create a subclass and extend my Importer-class.

  3. February 11th, 2011 at 15:54 | #3

    Plain simple! Use explicit values for your enum members that are NOT 0. Then just cast 0 to your enum and pass that value to your property.

    this.Importer = (Importer)0;

    Hope that helps.

  4. February 12th, 2011 at 16:35 | #4

    Enums are one of the few things where Java did a better job than C#.

    However code coverage is overrated, anyway :-)

  5. Anthony
    February 15th, 2011 at 14:05 | #5

    In Bill Wagner’s “Effective C#” he says that it’s a good idea for all enums to have a member “Unknown = 0″, or “Invalid = 0″ or similar. Otherwise, the first value in the enum will = 0. When an enum is not given an explicit value (e.g. unitialised field in a class type) it will get the zero value by default.

    So it’s good to have marker value at position zero so that you don’t act on an unitialised value that looks valid.

    .Net enums are partly-typesafe wrappers over integer types. As you have shown, it’s easy to push random int values in and out of them and no compile or runtime exception occurs. think it’s quite valid to do

    const int ValueOutOfRange = 142;
    this.Configuration.Importer = (Importer) ValueOutOfRange;

    It’s ugly, but it’s not as ugly as you make it out – I#s say it’s about as ugly as you’d expect a test that forces a out-of-range value into your code to be. Do it.

    I think it’s entirely wrong to say “enums are bad” – they have a role to store values that can take on one of a small set of named values. But when there’s significant behaviour, switched by that enum, add a class for each case. Especially when it’s a point where your code is growing. You can get a lot of extensibility and flexibility out of a Dictionary

  6. Anthony
    February 15th, 2011 at 14:08 | #6

    .. the last comment seems to have been mangled at the angle-brackets. Pity there’s no way to edit it or preview. The last bit should read something like
    can get a lot of extensibility and flexibility out of a Dictionary <SomeEnum, ISomeHandler >

  7. Anthony
    February 15th, 2011 at 14:17 | #7

    The problems of “using enums to define behavior” are not solved in any way by using constants. The problems of using constants to define behaviour are what enums where created to solve, specifically that
    – with an enum, you have to work hard to produce a value out of the expected range. With a string or int constant, it’s trivial and hard to spot.

    – With two or more sets of constants, it’s easy to entangle them, e.g.
    user.AccessLevel = VehicleType.Motorbike; may be valid and in range if “AccessLevel” and “Motorbike” are both string or int constants. With enums, you would not be able to assign a VehicleType to a AccessLevel, even if the numeric value was what you wanted.

    It’s true that when there is some significant behvaiour for each enum member, a class hierarchy is a better way to go than a switch statement.

  1. No trackbacks yet.