Thursday, September 3, 2009

On abstracting out static contexts

Any test favoring developer will tell you singletons are the bane of testability. Static contexts make testing difficult. When dealing with legacy code, we are often faced with the problem of testing around such problematic coupling. I've had enough opportunity to dable in such code to have developed some strategies in dealing with coupling. In this post I'll present a strategy I refer to as abstracting out singletons.

For the purpose of discussion I present the following code fragment:

public class TightlyCoupled {
 public void quickPurchase(String customerReferenceNumber, 
    int productCode, int quantity) {
  CustomerCatalog customers = CustomerCatalog.getInstance();
  Customer purchaser = customers.find(customerReferenceNumber);
  if (purchaser == null)
   throw new UnknownCustomerException(customerReferenceNumber);
  OrderItem requestedItems = Inventory.getInstance().take(productCode, quantity);
  Order quickOrder = new Order(purchaser);
  quickOrder.add(requestedItems);
  // ...
 }
}

This code is displays the symptoms of coupling that clearly make testing difficult. Can the inventory and customer catalog be easly programmed to behave in a predictable and testable manner? Your answer will be contextual, but often they cannot. In most enterprise application such singletons will bootstrap the initialization of other singletons as well as database connections. Can these services be easily mocked? Doubtful.

I have seen cases where programmers will allow themselves to override the singleton before the test scenario. This works, but does not help to move away from the current programming model.

What we want is to be able to provide an alternate implementation of these services to a constructed instance of the class under test (CUT). The problem with using a dependency injection approach to testing is that most applications developed with singleton spread are rarely blessed with an IoC container to facilitate object construction. Consequently we can rarely allow ourselves to remove the default constructor of the CUT.

So the first thing that I propose is that we consider using a DI approach, and in order to support this we provide two constructors for our CUT: one that allows for the injection of the dependencies, an a default constructor that wires the class with the default dependencies. This quickly leads us to something like this:

public class LessCoupledThroughConstructor {
 private CustomerCatalog customers;
 private Inventory inventory;

 public LessCoupledThroughConstructor(
   CustomerCatalog customers,
   Inventory inventory) {
  this.customers = customers;
  this.inventory = inventory;
 }

 public LessCoupledThroughConstructor() {
  this(CustomerCatalog.getInstance(), Inventory.getInstance());
 }

 public void quickPurchase(
  String customerReferenceNumber, 
  int productCode, int quantity) {
  Customer purchaser = customers.find(customerReferenceNumber);
  if (purchaser == null)
   throw new UnknownCustomerException(customerReferenceNumber);
  OrderItem requestedItems = inventory.take(productCode, quantity);
  Order quickOrder = new Order(purchaser);
  quickOrder.add(requestedItems);
  // ...
 }
}

So now our CUT can be passed a test friendly set of dependencies with a minimal impact on existing code. What's more, if the CUT is normally treated as a singleton also, its default constructor can become private, and its instance accessor would not take any dependencies. The test friendly constructor would remain public.

This approach has one drawback. When working on a legacy system that relied heavily on singleton accessors to initialize its services, I quickly discovered that many of these services had developed a kind of magical initialization order. No one knew for certain what that order was, but many bugs in the system were to it. This minor adjustment I proposed broke the initialization order: the dependency services where initialized in the constructor, and not on first use in the using method. Ka-boom!

No problem. The solution is to introduce a minimal amount of abstraction. I decided to creation some wrapper services. These services delegated operations to the static context on a per-call basis, but where not singletons or static instances themselves. This produces the code bellow:

public class LessCoupledThroughWraping {
 private CustomerCatalog customers;
 private Inventory inventory;
 
 public LessCoupledThroughWraping(
   CustomerCatalogService customers,
   InventoryService inventory) {
  this.customers = customers;
  this.inventory = inventory;
 }

 public LessCoupledThroughWraping() {
  this(new GlobalCustomerCatalogService(), new GlobalInventoryService());
 }

 public void quickPurchase(
   String customerReferenceNumber, 
   int productCode, int quantity) {
  // as previous example
 }
}

To give you an idea of what the wraper services look like:

public class GlobalInventoryService implements InventoryService {
 public OrderItem take(int productCode, int quantity) {
  return Inventory.getInstance().take(productCode, quantity);
 }
}

This approached allows the CUT to define a default set of dependencies, but still provides the possibility to inject dependencies.

These approaches are clearly not good examples of applied TDD, but on large systems that have a great deal of code coupling it is often necessary to introduce such test seams. Gradually a large system becomes split into an IoC container friendly design with well tested components.

Other areas where such a strategies work include all static data, static third party APIs, as well as static platform functinality.

An example that I used recently involves the .NET runtime's “DateTime.Now” which can make testing time dependent behavior unpredictable. My goal was to test that after 10 minutes some condition expired. Obviously I didn't want a test that ran for ten minutes. I abstract out a date and time service and at test time I substitute a date time service on which I can set the desired date and time. This makes testing cleaner and more reliable, and in this last case I only needed to redefine the date/time service's time to jump ahead ten minutes.

What do you think? Have you encountered similar problems? What did you do to work around them?

Tuesday, August 25, 2009

Agile 2009

well I havent posted anything in a while, but better late then never. I'm at the agile 2009 conference with my coleages from Pyxis Technologie. I've started posting on the conference on our corporate website. Check out: integration tests are a scam

Monday, April 20, 2009

On using containers as objects

Have you heard of the magic of IoC containers? if not go out there and educate yourself. I remember the first time someone told me about spring, the usefulness of it went clear over my head. It was almost a year before I had the chance to use one, and that first discovery was a revelation.

In fact, after that life-changing experience I wound up on a project where I could not add a 3rd party container, and was so devastated that I wrote a lightweight container-like service locator/factory.

I've since gone on to other projects with different containers, and have on a number of occasions used a trick that has proven useful a few times over.

Before spilling the beans, I have to warn you that I always hide the container behind some home rolled class. This allows me to be container independent - I like wrapping all external dependencies.

This allows me to do something a little unorthodox: upon container initialization, I have the container add itself to the set of possible services. Concretely this means that components that need the container to do something a little more elaborate then just resolve dependencies, can fall back on using the container as a "pull"-base locator.

How is this useful?

This pattern has proven useful for a number of occasions where it was handy to be able to select a service implementation based on some runtime condition.

Consider the following fictitious example where we select different implementations IDosomething identified by some identifier:

    if (a.Value == "a")
        IDosomething something = container.Resolve("a");
    else if (a.Value == "b")
        IDosomething something = container.Resolve("b");
    ...
    something.Do(a);
or simplified as
    IDosomething something = container.Resolve(a.Value);
    something.Do(a);

Because the different implementations of IDosomething are built by the container, they are subject to all container controlled lifecycles, aspects and DI.

Now if we wanted to inject the dispatching to IDosomething to the services that need it, we could abstract out the dispatching service (FYI in C# with Castle's Windsor container):

    class SomethingDispatcher {
        private ObjectContainer container;
       
        public SomethingDispatcher(ObjectContainer container) {
            this.container = container;
        }

        public void Do(SomeObject a) {
            IDosomething something = container.Resolve(a.Value);
            something.Do(a);
        }
    }

This handy idea has served me for configurable event processors on a number of occasions. It also saves me the mess of configuring in detail the injection of each handler.

So how di I get the container in the container? With the following type of hack:

    class ObjectContainer {
        ObjectContainer() {
            this.container = ....
            this.container.Kernel.AddComponentInstance(this);
        }
        ...

In some cases it can be useful to specify qualified service IDs like "a.eventHandler" and "event-5633.eventProcessor". If this is the case, the dispatcher can take charge of building the qualified IDs it supports for example:

        string doerId = string.Format("{0}.eventHandler", a.Value);
and
        string eventId = string.Format("event-{0}.eventProcessot", eventCode);

Wednesday, March 18, 2009

On inverting control

The term inversion of control is one that gets bandied around quite easily these days. Unfortunately most people react to the term with blank stares. In the hopes of preparing my explanation for the next time, here is my reflection on IoC. Consider the simple example:
class CustomerInfo {
}

interface NotificationService {
 void sendNotification(CustomerInfo info);
}
this separation is logical because the sending of customer info is not necessarily bound to the sending of some notice. It does however have a few problems: for one it forces each implementation of the notification service to know the details of the customer info. We can however consider some inversion on this:
class CustomerInfo {
 void sendTo(NotificationService notificationService);
}

interface NotificationService {
 void sendNotification(String message);
}
this would result in a send command that looks like this:
customerInfo.sendTo(notificationService);
which reads fairly well. This inversion has helped us to simplify the notification service, and allowed us to re-encapsulate the customer info. Now what happens if the notice has a complicated, transport-specific format? We can make the abstraction more transparent flexible:
interface NotificationService {
 NotificationMessage createMessage();
 void sendNotification(NotificationMessage message);
}

interface NotificationMessage {
 void setSubmiter(String submiter);
 void setSubject(String subject);
 void setMessage(String message);
}

class CustomerInfo {
 void sendTo(NotificationService notificationService) {
  NotificationMessage message = notificationService.createMessage();
  ...
  notificationService.sendNotification(message);
 }
}
Now this approach could allow us to implement an email-based notification or an instant-messaging-based notification service. The code is cleaner and safer, all without coupling the service to the source of the message. Not bad.

Saturday, March 14, 2009

On the smallest possible condition

Consider how 'if' statement logic can frequently obscure the behavior of the code.

Frequently a simple 'if' statement can contain two to three conditions. Sometimes more, rarely less.

This results in hard to read and maintain conditions. In some cases a condition can have two or more sets of logical conditions.

As an example, consider the following simple condition:

if (name != null && name.length < 20 && address.country == "CA") {
 ...
}

As programmers we may recognize that there are two intended conditions here. One deals with the validity of the name, while the other deals with the address being a domestic address (if you're in Canada). In trying to clean up this code we will frequently break this 'and' of two conditions into two 'if' statements like so:

if (name != null && name.length < 20) {
 if (address.country == "xyz") {
  ...
 }
}

this strikes me as no more clear than the first case. The dual 'if' statements correctly identify the two conditions, but their separation does not clarify the meaning of the code.

In my mind this clean up is heading in the wrong direction. I propose we take a step back to the original example.

If the conditions can be named in a descriptive way, then maybe we can clear this up a little. I propose that we create methods that will express the two conditions separately. I propose isNameValid() and isDomesticAddress().

This should result in the following condition:

boolean isNameValid() {
 return name != null && name.length < 20;
}

boolean isDomesticAddress() {
 return address.country == "xyz";
}

...
if (isNameValid() && isDomesticAddress()) {
 ...
}

This now speaks volumes more than either of the previous examples. Firstly in defines a clear logical combination of conditions.

Arguably we could have applied the same refactoring to the two-if structure, and it would have looked like this:

if (isNameValid()) {
 if (isDomesticAddress()) {
  ...
 }
}

I don't think this helps clarify things as much as the combined if statement.

In following with the idea of condition methods, the and-ed content of the condition in the third code snippet should be regrouped as:

boolean isValidForDomesticShipping() {
 return isNameValid() && isDomesticAddress();
}

and subsequently the conditional would look like this:

if (isValidForDomesticShipping()) {
 ...
}

Now the isValidForDomesticShipping can be easily tested on its own, and the if statement reads much better.

Negations

Now continuing along the road to conditional clarity, let us consider the impact of using negative conditions. In C-style languages such as C++, C# and Java, the negation of a boolean expression is done with the use of the exclamation point. Now the simplicity of this syntax is great, except that it really doesnt jump out when speed reading the code. Obviously Python doesnt suffer from this problem, the negation is a resounding three letter operator "not".

An if statements should not use the negative condition to inverse the behavior of the conditional method. Instead it is clearer to write a conditional method that represents the expressed inversion. This forces the method to say what the condition means.

isNotValidForShipping stands out more clearly than !isValidForShipping.

Help, I'm drowning in methods

As the number of conditional methods increases, you may find your classes getting a little cluttered. This may be a sign that the classes have too many members.

in order to unclutter a class, pushing some of its members and corresponding methods (including conditional methods) into smaller classes.

This means that instead of a string member called name, your Shipping slip may have a member of type CustomerName, and as well as a member of type Address.

An example of the above code with delegated responsibility could resemble this:

boolean isValidForDomesticShipping() {
 return customerName.isValid() && shippingAddress.isDomestic();
}

Now this is starting to make the code cleaner and clearer still.

Conclusion

Rules

  1. 'if' statements should not include conditions, only a call to a conditional method.
  2. As much as possible, an if statements should not use the negative condition to inverse the behavior of the conditional method. Instead it is clearer to write a conditional method that represents the negation in plain English (or whatever language you're programming in)
  3. A conditional method should use only one comparison operation (!=, >, <, ==, etc) or one combining operation (&&, ||). This forces combined conditions to use condition methods on the individual members.
  4. Conditional operations should never have 'if' statements themselves, they should be composed of logical operators and conditional method calls only.
  5. Complex condition methods should be broken into a number of conditional methods.

This approach to condition evaluation helps to make code self-documenting. It can also facilitate testing of conditional logic as it separates the evaluation of the condition from the if statement.

If I start to find that a class is accumulating too many conditional methods, then chances are that the class does too much, and some of its members need to be split off into new classes, the conditional methods will follow.

Friday, February 13, 2009

On testing data objects, applied TDD

In a previous post I discussed how you might approach unit testing a data object. In a test-first approach to development, we first write the tests (obviously), and then write the simplest code that makes the test pass. In this exercise I will propose the tests, writing the code will be up to you. This exercise is designed to be written in Java with JUnit 3, but can be easily adapted to another xUnit framework and language.

The context of the exercise is the reservation of tables at a fair. A person can reserve one or more tables on which to present their offerings.

First lets imagine we want to create a reservation for a single table by 'bob'.

public void testSingleTableReservationBelongsToReserver() {
 Reservation bobsReservation = Reservation.forSingleTable("bob");
 assertEquals("bob", bobsReservation.getReservedBy());
}

Ask yourself what the simplest code to implement the reservation class would look like, and write it. Make sure your test passes.

Next we will validate that the reservation is specifically for one table. This is another independent test since I want to be able to easily identify the source of failure if a test fails. Chaining multiple assertions in a single test can cause one failure to hide another.

public void testSingleTableReservationReservesOneTable() {
 Reservation bobsReservation = Reservation.forSingleTable("bob");
 assertEquals(1, bobsReservation.getNumberOfTables());
}

Again, implement the simplest code to make the test pass. In this case the method getNumberOfTables could return the constant '1'.

Next let's validate that the cost of the reservation is the sum of the registration fee (30$) and the cost per table (20$). Again this method could return a hard-coded constant.

public void testCostOfSingleTableReservationIsCostPerTablePlusRegistrationFee();
 Reservation bobsReservation = Reservation.forSingleTable("bob");
 assertEquals(50, bobsReservation.getTotalReservationCost());
}

Next let's do a negative test for the creation of a multi-table reservation. The maximum number of tables allowed is 4 per person.

public void testCannotCreateReservationForMoreThanMaxAllowedTables() {
 try {
  Reservation.forMultipleTable("jim", 5);
  fail();
 } catch (TooManyTablesRequestedException a) {
 }
}

It seems that Jim's out of luck. This test forced us to create a new factory method as well as a new exception. If your new method has an if statement, you've already gone too far. At this point your method should only contain a single statement: a throw new exception. I know it's painfuly, small step to many newbies, but we are working our way up slowly as would be done in TDD. Its is an incremental development process.

Ok, maybe Jim can settle for reserving 4 tables.

public void testCreateReservationForMaximumNumberOfTables() throws Exception {
 Reservation joesReservation = Reservation.forMultipleTable("joe", 4);
 assertEquals(4, joesReservation.getNumberOfTables());
}

So far things are moving along well. Our business rules for the calculation of the cost of the reservation may need some elaborating. Lets ask for the cost of reserving two tables.

public void testCostOfReservationForTwo() throws Exception{
  Reservation chrisReservation = Reservation.forMultipleTable("chris", 2);
  assertEquals(70, chrisReservation.getTotalReservationCost());
}

At this point we may apply a 10% discount to Jim's reservation because he's reserving 4 tables.

public void testCostOfReservationForFourTablesIncludesDiscount() throws Exception{
  Reservation joesReservation = Reservation.forMultipleTable("joe", 4);
  assertEquals(99, joesReservation.getTotalReservationCost());
}

We will now add two tests that will target the updating of the data object's numberOfTables property.

public void testChangeNumberOfTablesReservedOverridesPreviousNumber() throws Exception {
  Reservation joesReservation = Reservation.forMultipleTable("joe", 4);
  joesReservation.setNumberOfTables(3);
  assertEquals(3, joesReservation.getNumberOfTables());
}
 
public void testChangeNumberOfTablesReservedChangesCostOfReservation() throws Exception {
  Reservation joesReservation = Reservation.forMultipleTable("joe", 4);
  joesReservation.setNumberOfTables(1);
  assertEquals(50, joesReservation.getTotalReservationCost());
}

In many baby steps we will have developed a simple, well tested implementation of Reservation. This particular exercise is trivial, and did not push us to do any complicated refactoring. In practice we would want to follow the tree step TDD cycle: red, green, refactor.

Usually the hardest part to test-first development is imagining what to test. I hope this exercise has shed some light on how to start the test writing.

Sunday, February 8, 2009

On handling exceptions in happy path tests

I'd like to start this post with a reflection on the goal of unit testing.

The goal of unit tests is to validate the single, precise behavior of a block of code. This indicates that a test's value is directly dependent on its ability to spot deviations in that behavior.

So lets look at a simple test, the likes of which I have seen before:

void testSomeBizareCondition() {
 try {
  op.someOperartionThatShouldBeOK();
  assertEquals("abc", op.getValue());
 } catch (NullPointerException e) {
  fail("oops");
 } catch (SomeOperationException e) {
  fail("oops");
 }
}

What is wrong with this test? If the test should fail with an "oops" there is no way to differentiate between the two error conditions. Instead of reading the test report logs and quickly correcting the code, some developers will find themselves debugging the tests. The answer to that is not to change the failure messages for each catch, because this would lead us down the path of catching every imaginable exception type.

Another reason not to favor catching of individual exception types is illustrated bellow:

void testSomeBizareConditionThrowsAnOperationException() {
 try {
  op.someOperartionThatShouldBeOK();
  fail("oops");
 } catch (NullPointerException e) {
  fail("oops");
 } catch (SomeOperationException e) {
 }
}

At a glance, can you tell what the goal of the test is? Maybe you can, but it isn't obvious, and you shouldn't have to thing about it. This test is intended to validate that SomeOperationException is thrown during test execution. Unfortunately this intention is not evident.

In order to illustrate the solution to both of the above tests, lets look at the basic concept. Bellow is a test that represents a simplified version of the first example.

void testSomeOptimisticCondition() {
 try {
  op.someOperationThatShouldBeOK();
  assertEquals("abc", op.getValue());
 } catch (Trowable th) {
  fail("bad");
 }
}

This example still masks the failure reason, the cause of exception is not in evidence. The stack trace and exception details go a long way to facilitating a quick code correction.

A second problem with this test is that it masks the details of the assertion failure. This could be corrected by changing the catch type from Throwable to Exception. This is because an assertion failure is expressed in the form of an exception.

The solution

Its hard to imagine, but sometimes the best thing to do is let the exception propagate out of the test. For unchecked exceptions this is easy, the test will fail, and the cause will be plain as day in the test output.

For checked exceptions this is a little more complicated, but only slightly. Let the test method throw a generic exception.

void testSomeOptimisticCondition() throws Exception {
 op.someOperationThatShouldBeOK();
 assertEquals("abc", op.getValue());
}

Not only does the test now guaranty that all failure conditions are now explicit, it makes the test a lot clearer.

Test cleanup

Now all this discussion on removing clutter may well have some readers thinking: "yeah, but I use a catch to cleanup after the test". It is true that some tests require the cleaning of external resources. The problem with doing so in a try/catch is that it can complicate test logic. There are two viable solutions, one is to use a try/finally, putting the cleanup in the finally, and another is to put the logic in the tear down method. I tend to favor the tear-down, because it removes clutter from the test method.

Test should always fail fast. So how do we handle exceptions in happy-path tests? We don't.