Congratulations to Michaël Willemot who is the winner of
Challenge 2! Michaël has won a copy of
Applying Domain-Driven Design and Patterns: With Examples in C# and .NET by Jimmy Nilsson along with a 3-user license pack for
CodeIt.Once Refactoring. (The winner was randomly selected from all correct entries received.) Geert Baeyaert had the first correct submission and, incidentally, the first correct bonus entry as well.
There were 19 entries altogether with 17 correct entries. 11 entries attempted the bonus but only 5 entries correctly completed the bonus with a mock logger. Here's a run down of the correct entrants: Adam Vandenberg (bonus as well), Andrzej Kuczynski, Avi A, Bill Arnette, Brian Sherwin, Franck Tetard, Geert Baeyaert (first entry and bonus as well), Harry Chou (bonus as well), John Penokie, Jonathan Roe, Joshua McKinney (bonus as well), Matt Payne, Michael Willemot, Navneet Gupta, Rasmus Faber-Espensen, Rune Rystad (last week's winner, so not eligible for prize, but got bonus as well), Vitaly Davidovich. Nice work all!
Interesting FindsBelow are a few items of particular interest that were found in the submissions:
- A few entrants included an abstract "join spec" that acted as the parent for the AndSpec, NotSpec and OrSpec. This helped centralize functionality and reduce duplicated code. Furthermore, a couple of entrants also made the spec pattern completely generic so its structure could be easily reused with different domain objects. Great call on both approaches.
- I forgot to mention, and I'm glad I did, to remove the two unit tests that were affected when you removed the methods FindByNotTypeAndAtLeastTotalCost and FindAtLeastQuantityOrAtLeastTotalCost. Quite a few people wondered whether they should be left alone, modified, or deleted. I'm glad it brought up this unintended conversation. Unit tests should be seen as a dynamic, living part of the code and are subject to the same level of refactoring as the "real" code. If a unit test is no longer pulling its weight or is no longer applicable, get rid of it. As Brian Sherwin noted, a great book to take a look at is Test-Driven Development by Kent Beck. This book demonstrates just how dynamic unit test development really is. With full-throttle TDD, the unit tests morph as much, if not more, as the production code itself.
- The log4net bonus tripped up entrants mainly due to two reasons: 1) two separate logs needed to be generated, as was working in the original code, not just one, and 2) the singleton methods were tricky to mock. One solution to the first trip-point was to pass two loggers, one for the owner's log and one for the auditor's log to the nightly log process. The drawback to this approach is the maintenance headache with having to later on pass a third, forth, or nth logger to the class or to another class altogether. Alternatively, another approach is to supply a simplified Service Locator. This "logger locator" could be easily passed to anything and extended easily. I've attached a zip file, ExampleSolutions.zip, which contains four entries which used this type of approach (and contain a few variants of the spec pattern as well). The second trip-point was dealing with mocking the call to a method on a singleton. You can make a wrapper for the object, or, if you're in a real pickle, you can use a tool such as TypeMock which can mock just about anything, even sealed classes. The drawback to using a tool such as this is that it makes it easier to write more tightly coupled code. As mentioned in a previous post, Michael Feather's Working Effectively with Legacy Code has a number of great ideas for dealing with these types of situations without resorting to such a powerful tool. (You saw what the ring of power did to Frodo, didn't you?)
- Someone brought up the question of what to do if you find yourself writing duplicated spec-creation code in different areas of the code. One solution is to create concrete "combination spec" classes for each of these variants. The problem with this is that it leads to the same type of combinatorial class explosion that we were running into before. Alternatively, for the duplicated spec combinations, you could create a CombinationSpecFactory class. This would have one or more creation methods that would return a Spec class; one method for each unique combination which is duplicated in more than one place. You might ask, what's better about having a factory with combination-spec-creation methods vs. having a number of concrete-combination spec classes? My preference of a factory over individual classes would be to keep the Spec class package clean; I want to keep my Spec class names to be as atomic as possible. I'd love to hear other thoughts on this issue!
Instead of describing the smell refactorings in detail, I'll let the code speak for itself. I've attached ExampleSolutions.zip below which contains four projects that have the bonus challenge included along with well-written log4net wrappers. Your comments are most welcome as to pros and cons of each approach.
I hope these refactoring challenges continue to be useful, offering plenty of food for thought. Stay tuned to the next challenge which will be rearing its ugly head sometime next week!
Billy