Geeks With Blogs

News Meta
Gruff Code Code and tech musings from Jesse Taber. If I call it a blog I'll feel bad when I don't update it every week.

My coworker John Sonmez had a great blog post recently illustrating how to refactor a switch statement. In short, he outlined how to take a switch statement covering all known values of a C# Enumeration and convert it into a Dictionary “map” object allowing you to quickly determine what action to take given any valid enumeration value. I really like this approach as I think it leads to more readable code. The one thing that’s always bothered me about enumerations though is that they can change with your codebase over time. Renaming or removing an enumeration will likely give you some easily detected compile-time errors, but adding a new enumeration value can lead to bugs that might not be able to detect until runtime. Errors like this can easily make their way into production environments and cause big problems. I’d much rather catch this issue earlier. I couldn’t think of any way to catch issues like this at compile time, but if you’re writing automated unit/integration tests for your application and having them run periodically (which you should be), then the “map” object approach will let you write tests to catch these issues before they make it out the door.

An Example

I’m going to steal borrow the example from John’s post to illustrate how this might work. Let’s assume that in this example we have an enumeration containing four values representing different file formats that might be used when a file is provided from some external source and needs to be processed. The first step in processing the file will be to parse it and to correctly parse it we need to know what format it is in. Let’s start with the definition of the enumeration:

public enum FileType { Pdf, Image, CsvWithHeaderRow, Csv }

We need to define a class to parse each of the four file types. Rather than having a switch statement to do this, John’s post shows how you could use a Dictionary to create a map instead. Let’s say that we’re going to wrap all of this up in a ‘FileParserFactory’:

public class FileParserFactory { private static Dictionary<FileType, Func<DataFile, DataFileParser>> FileParserMap = new Dictionary<FileType, Func<DataFile, DataFileParser>> { {FileType.Pdf, file => new PdfParser(file)}, {FileType.Image, file => new ImageParser(file)}, {FileType.CsvWithHeaderRow, file => new CsvParser(file)}, {FileType.Csv, file => new CsvParser(file)} }; public DataFileParser GetFileParserFor(DataFile file) { if (!FileParserMap.ContainsKey(file.FileType)) throw new ArgumentOutOfRangeException("Unexpected file type value " + file.FileType); return FileParserMap[file.FileType](file); } }

Our map object contains a valid action to take for all four of our expected file formats. Let’s say now that a customer comes along and wants to be able to upload files in Excel format without having to first convert it to CSV. This task gets assigned to a developer who writes the parsing logic for the Excel file format and adds ‘Excel’ as a fifth value in the ‘FileType’ enumeration, but forgets to add the mapping between the new parser class and the new enumeration value above.In this admittedly contrived example this issue would likely be caught very quickly by a round of QA testing, but I think that we can catch this oversight even earlier than that.

Testing the Mapping

What I want to be able to do is write a unit test that will ensure that the mapping dictionary above contains a mapping for every defined value of the ‘FileType’ enumeration. My first thought here was to use reflection and a little bit of Linq to make this work. If we want to assume that the FileParserFactory class will be constructed at least once during the normal run of unit tests for your application, we could define a constructor for the FileParserFactory and validate the enumeration mappings there:

public FileParserFactory() { var unmappedFileTypes = Enum.GetValues(typeof (FileType)) .Cast<FileType>() .Where(fileType => !FileParserMap.ContainsKey(fileType)) .ToList(); if (unmappedFileTypes.Count > 0) throw new ApplicationException("One or more file types are not mapped."); }

Here in the constructor we’re grabbing all of the values defined by the ‘FileType’ enumeration, then finding which of them (if any) are not defined as keys in the dictionary. If we find any that aren’t mapped, we throw an exception. While this approach might work, I don’t think it’s a great solution. First of all, this makes the assumption that your unit tests are going to be invoking the constructor of the FileParserFactory. If you have decent code coverage this is probably an OK assumption to make, but it’s an assumption nonetheless. Secondly, I don’t think that logic like this belongs in the constructor of a class like this. Constructors should be doing things to setup the object for use, not performing validation like this. Lastly, running this bit of code as every instance of the FileParserFactory is wasteful and adds unnecessary overhead to the application. I really only care about running this validation when unit tests run, so I’d rather refactor that code out of here and into a test.

Exposing the Mapping Details

One quick and dirty way to accomplish what I want would be to change the declaration of my map object from ‘private’ to ‘public’. That way I could write a test to validate the completeness of the mapping from an external test like this:

[Test] public void should_define_mapping_for_all_potential_file_types() { var unmappedFileTypes = Enum.GetValues(typeof(FileType)) .Cast<FileType>() .Where(fileType => !FileParserFactory.FileParserMap.ContainsKey(fileType)) .ToList(); Assert.AreEqual(unmappedFileTypes.Count, 0); }

While this is better because we moved this validation logic into a test, but we had to expose a bit of the FileParserFactory that really shouldn’t be exposed. If we make the dictionary public we’d be allowing any potential consumer of the class to arbitrarily define new mappings or even change existing mappings.

Instead, I want to expose a read-only collection of the mapped FileType values from the FileParserFactory like this:

public static IEnumerable<FileType> MappedFileTypes { get { return FileParserMap.Keys; } }

With this collection of FileType values exposed, we just need to tweak our unit test a bit to make it work against this property as opposed to the dictionary itself:

[Test] public void should_define_mapping_for_all_potential_file_types() { var unmappedFileTypes = Enum.GetValues(typeof(FileType)) .Cast<FileType>() .Where(fileType => !FileParserFactory.MappedFileTypes.Contains(fileType)) .ToList(); Assert.AreEqual(unmappedFileTypes.Count, 0); }

Room For Improvement

At this point I think I should say that I’m not necessarily advocating testing all mappings in every application. In some cases it may be perfectly acceptable to have mappings for some enumeration values and not others. That said, I do think it’s important that you have tests to validate the expected behavior that suits your particular scenarios. For cases where you need to ensure that you have a mapping in place for all possible enumeration values I think that the approach outlined above is a good start, but there’s definitely some room for improvement. At this point we’re exposing an IEnumerable of FileType values solely for the purpose of testing our mappings. I suppose one could make the argument that this is just the cost of doing business when writing testable code, but I really think that we can make this cleaner. I’ll dig into this more in a future post.

Posted on Tuesday, November 2, 2010 7:59 PM | Back to top

Comments on this post: Testing Enumeration Mappings

# re: Testing Enumeration Mappings
Requesting Gravatar...
The other thing with enumeration is give values to the enums, if tommorow you delete or reorder they will be still be working the same.
Left by biswanaths on Nov 03, 2010 5:03 AM

Your comment:
 (will show your gravatar)

Copyright © Jesse Taber | Powered by: