Friday, March 30, 2007

Making Classes Unit-Testable

I had been working on the code review of one of our Java projects, when the following snippet struck me as a definite smell in one of the POJOs :


class TradeValueCalculator {
  // ..
  // ..

  public BigDecimal calculateTradeValue(final Trade trade, ..) {
    // ..
    BigDecimal tax = TradeUtils.calculateTax(trade);
    BigDecimal commission = TradeUtils.calculateCommission(trade);
    // .. other business logic to compute net value
  }
  // ..
  // ..
}



What is the problem with the above two innocuous looking Java lines of code ? The answer is very simple - Unit Testability of the POJO class TradeValueCalculator ! Yes, this post is about unit testability and some tips that we can follow to design classes that can be easily unit tested. I encountered many of these problems while doing code review of a live Java project in recent times.

Avoid Statics

When it comes to testability, statics are definitely not your friends. In the above code snippet, the class TradeValueCalculator depends on the implementation of the static methods like TradeUtils.calculateTax(..) and TradeUtils.calculateCommission(..). Any change in these static methods can lead to failures of unit tests of class TradeValueCalculator. Hence statics introduce unwanted coupling between classes, thereby violating the principle of easy unit-testability of POJOs. Avoid them, if you can, and use standard design idioms like composition-with-dependency-injection instead. And while using composition with service components, make sure they are powered by interfaces. Interfaces provide the right level of abstraction for multiple implementations and are much easier to mock while testing. Let us refactor the above snippet to compose using service components for calculating tax and commission :


class TradeValueCalculator {

  // .. to be dependency injected
  private ITaxCalculator taxCalculator;
  private ICommissionCalculator commissionCalculator;

  // ..
  // ..

  public BigDecimal calculateTradeValue(final Trade trade, ..) {
    // ..
    BigDecimal tax = taxCalculator.calculateTax(trade);
    BigDecimal commission = commissionCalculator.calculateCommission(trade);
    // .. other business logic to compute net value
  }
  // ..
  // ..
}

interface ITaxCalculator {
  BigDecimal calculateTax(..);
}

interface ICommissionCalculator {
  BigDecimal calculateCommission(..);
}



We can then have concrete instances of these service contracts and inject them into the POJO TradeValueCalculator :


class DefaultTaxCalculator implements ITaxCalculator {
  // ..
}

class DefaultCommissionCalculator implements ICommissionCalculator {
  // ..
}



Using standard IoC containers like Guice or Spring, we can inject concrete implementations into our POJO non-invasively through configuration code. In Guice we can define Modules that bind interfaces to concrete implementations and use Java 5 annotation to inject those bindings in appropriate places.



// define module to configure bindings
class TradeModule extends AbstractModule {

  @Override
  protected void configure() {
  bind(ITaxCalculator .class)
      .to(DefaultTaxCalculator .class)
      .in(Scopes.SINGLETON);

    bind(ICommissionCalculator .class)
      .to(DefaultCommissionCalculator .class)
      .in(Scopes.SINGLETON);
  }
}



and then inject ..


class TradeValueCalculator {

  // ..
  @Inject private ITaxCalculator taxCalculator;
  @Inject private ICommissionCalculator commissionCalculator;

  // ..
  // ..
}



How does this improve testability of our class TradeValueCalculator ?

Just replace the defined Module by another one for unit testing :


// define module to configure bindings
class TestTradeModule extends AbstractModule {

  @Override
  protected void configure() {
    bind(ITaxCalculator .class)
      .to(MockTaxCalculator .class)
      .in(Scopes.SINGLETON);

    bind(ICommissionCalculator .class)
      .to(MockCommissionCalculator .class)
      .in(Scopes.SINGLETON);
  }
}



What we have done just now is mocked out the service interfaces for tax and commission calculation. And that too without a single line of code being changed in the actual class! TradeValueCalculator can now be unit-tested without having any dependency on other classes.

Extreme Encapsulation

I have come across many abuses of FluentInterfaces, where developers use chained method invocations involving multiple classes. Take this example from this Mock Objects paper, which discusses this same problem :

dog.getBody().getTail().wag();

The problem here is that the main class Dog is indirectly coupled with multiple classes, thereby violating the Law of Demeter and making it totally unsuitable for unit testing. The situation is typically called "The Train Wreck" and has been discussed extensively in the said paper. The takeway from this situation is to minimize coupling with neighbouring classes - couple only with the class directly associated with you. Think in terms of abstracting the behavior *only* with respect to the class with which you collaborate directly - leave implementation of the rest of the behavior to the latter.

Privates also need to be Unit-Tested

There is a school of thought which espouses the policy that *only* public api s need to be unit-tested. This is not true - I firmly believe that all your methods and behaviors need unit testing. Strive to achieve the maximum coverage of unit testing in your classes. Roy Osherove thinks that we may have to bend some of the rules of pure OOD to make our design implementations more testable e.g. by exposing or replacing private instances of objects using interfaces, injection patterns, public setters etc. Or by discouraging default sealing of classes allowing overriding in unit tests. Or by allowing singletons to be replaced in tests to break dependencies. I think, I agree to many of these policies.

Fortunately Java provides a useful access specifier that comes in handy here - the package private scope of access. Instead of making your implementation members *private*, make them *package private* and implement unit test classes in the same package. Doing this, you do not expose the private parts to the public, while allowing access to all unit test classes. Crazy Bob has more details on this. Another useful trick to this may be usage of AOP. As part of unit test classes, you can introduce additional getters through AOP to access the implementation artifacts of your class. This can be done through inter-type declarations, and the test classes can access all private data at gay abandon.

Look out for Instantiations

There are many cases where the class that is being unit tested needs to create / instantiate objects of the collaborating class. e.g.


class TradeController {

  // ..
  // ..

  public void doTrade(TradeDTO dto, ..) {
    Trade trade = new Trade(dto);
    // .. logic for trade
  }
  // ..
}



Increase the testability of the class TradeController by separating out all creation into appropriate factory methods. These methods can then be overridden in test cases to inject creation of Mock objects.


class TradeController {
  TradeDTO dto;

  // ..
  // ..

  public void doTrade() {
    Trade trade = createTrade(dto);
    // .. logic for trade
  }
  // ..

  protected Trade createTrade(TradeDTO dto) {
    return new Trade(dto);
  }
}



and create MockTrade in test cases ..


class TradeControllerTest extends TestCase {

  // ..

  public void testTradeController(..) {
    TradeController tc = new TradeController() {
      protected Trade createTrade(TradeDTO dto) {
        return new MockTrade(dto);
      }
    }
    tc.doTrade();
  }
}



The Factory Method pattern proves quite helpful in such circumstances. However, there are some design patterns like The Abstract Factory, which can potentially introduce unwanted coupling between classes, thereby making them difficult to unit-test. Most of the design patterns in GOF are built on composition - try implementing them using Interfaces in Java, so that they can be easily mocked out. Another difficult pattern is the Singleton - I usually employ the IoC container to manage and unit-test classes that collaborate with Singletons. Apart from static methods, which I have already mentioned above, static members are also problematic cases for unit testing. In many applicatiosn they are used for caching (e.g. ORMs) - hence an obvious problem child for unit testing.

Wednesday, March 21, 2007

Using Guice as the DI Framework - some hiccups

Finally got the time to lay my hands on Guice, the new IoC container from Google. I ported one of my small applications based on Spring to Guice - nothing much too complicated, but just to explore the features of Guice and some user level comparison with Spring. I have been a long time Spring user (and an admirer too), hence the comparison is just an automatic and involuntary sideeffect of what I do with any of the other IoC containers on earth.

My first impression with Guice has been somewhat mixed. It is really refreshing to work with the extremly well-designed api's, packed with the power of generics and annotations from Java 5. Fluent interfaces like binder.bind(Service.class).annotatedWith(Blue.class).to(BlueService.class)
make ideal DSLs in Java and give you the feeling that you are programming to Guice rather than to Java. This is a similar feeling that you get when programming to Rails (as opposed to programming in Ruby). However, I came across some stumbling blocks which have been major irritants for the problem that I was trying to solve. Just to point out that I have only fiddled around with Guice for a couple of days, and may have missed out lots of details which can offer better solutions to the problems. Any advice, suggestions from the experts will be of great help. Here are some of the rants from my exercise with porting an existing application into Guice :

A Different way to look at Configuration

Many people have blogged about the onerous XML hell of Spring and how Guice gets rid of these annoyances. I think the main difference between Guice and Spring lies in the philosophy of how they both look at dependencies and configuration. Spring preaches the non-invasive approach (my favorite) and takes a completely externalized view towards object dependencies. In Spring, you can either wire up dependencies using XML or Spring JavaConfig or Groovy-Spring DSL or some other option like using Spring-annotations. But irrespective of the techniques you use, dependencies are always externalized :


@Configuration
public class MyConfig {
  @Bean
  public Person rod() {
    return new Person("Rod Johnson");
  }

  @Bean(scope = Scope.PROTOTYPE)
  public Book book() {
    Book book = new Book("Expert One-on-One J2EE Design and Development");
    book.setAuthor(rod()); // rod() method is actually a bean reference !
    return book;
  }
}



The above is an example from Rod Johnson's blog post - the class MyConfig is an externalized rendition of bean configurations. It uses Java 5 annotations to define beans and their scopes, but, at the end of the day, all it does is equivalent to spitting out the following XML :


<bean id="rod" class="Person" scope="singleton">
  <constructor-arg>Rod Johnson</constructor-arg>
</bean>

<bean id="book" class="Book" scope="prototype">
  <constructor-arg>Expert One-on-One J2EE Design and Development</constructor-arg>
  <property name="author" ref="rod"/>
</bean>



Guice, on the other hand, treats configuration as a first class citizen of your application model and allows them right into your domain model code. Guice modules indicate what to inject, while annotations indicate where to inject. You annotate the class itself with the injections (through @Inject annotation). The drawback (if you consider it to be one) is that you have to import com.google.inject.* within your domain model. But it ensures locality of intentions, explicit semantics of insitu injections through metadata programming.


// what to inject : a sample Module
public class TradeModule extends AbstractModule {
  protected void configure() {
    bind(Trade.class).to(TradeImpl.class);
    bind(Balance.class).to(BalanceImpl.class);
    bindConstant().annotatedWith(Bond.class).to("fixed income");
    bindConstant().annotatedWith(I.class).to(5);
  }
}

// where to inject : a sample domain class
public class TradingSystem {
  @Inject Trade trade;
  @Inject Balance balance;

  @Inject @Bond String tradeType;

  int settlementDays;

  @Inject
  void setSettlementDays(@I int settlementDays) {
    this.settlementDays = settlementDays;
  }
}



Personally I would like to have configurations separated from my domain code - Guice looked to be quite intrusive to me in this respect. Using Spring with XML based configuration allows a clean separation of configuration from your application codebase. If you do not like XML based configurations, use Spring JavaConfig, which restricts annotations to configuration classes only. Cool stuff.

Annotations! Annotations!

Guice is based on Java 5 annotations. As I mentioned above, where-to-inject is specified using annotations only. The plus with this approach is that the intention is explicit and locally specified, which leads to good maintenability of code. However, in some cases, people may jump into overdose of annotations. Custom annotations should be restricted to minimum and should be used *only* as the last resort. Guice provides a Provider<T> abstraction to deal with fine grained instantiation controls. In fact Provider<T> is an exceptionally simple abstraction, but can be used very meaningfully to implement lazy variants of many design patterns like Factory and Strategy. In my application I have used Provider<T> successfully in implementing a Strategy, which I initially implemented using custom annotations. Lots of custom annotations is a design smell - try refactoring your design using abstractions like Provider<T> to minimize them.

Problems with Provider<T>

However, I hit upon a roadblock while implementing some complex strategies using Provider<T>. In many cases, my Strategy needed access to contextual information in order to decide upon the exact concrete strategy to be instantiated. In the following example, I need different strategy instances of CalculationStrategy depending on the trade type.


interface CalculationStrategy {
  void calculate();
}

public class TradeValueCalculation {

  private CalculationStrategy strategy;
  private Trade trade;

  // need different instances of strategy depending on trade type
  public TradeValueCalculation(Trade trade, CalculationStrategy strategy) {
    this.trade = trade;
    this.strategy = strategy;
  }

  public void calculate() {
    strategy.calculate();
  }
}



I cannot use any custom annotation on constructor argument strategy, since I need the polymorphic behavior for different instances of the same class. I tried with the Provider<T> approach :


public class TradeValueCalculation {

  private Provider<CalculationStrategy> strategy;
  private Trade trade;

  @Inject
  public TradeValueCalculation(Trade trade, Provider<CalculationStrategy> strategy) {
    this.trade = trade;
    this.strategy = strategy;
  }

  public void calculate() {
    strategy.get().calculate();
  }
}



Still the Provider does not have the context information .. :-( and my problem is how to pass this information to the Provider. Any help will be appreciated ..

On the contrary, solving this in Spring is rather simple by declaring multiple bean configurations for the same class. And this works like a charm in the XML as well as the Java variant of configuring Spring beans.

Some other pain points ..


  • Guice user guide recommends using Provider<T> to inject into third party classes. This looked quite obtuse to me since it goes against the philosophy of less code that Guice preaches. Spring provides much elegant solutions to this problem because of its non-invasiveness property. Guice, by virtue of being an intrusive framework, had to provide this extra level of indirection to DI into classes for which I do not have the source code.


  • One specific irritant in Guice is the literal injection, which forced me to use a custom annotation everytime I wanted to inject a String literal.


  • Another feature which would have been very useful for me is the ability to override bindings through Module hierarchies. In one of my big applications, I have multiple components where I thought I can organize my Guice Modules in a similar hierarchy with specific bindings being overridden in specific modules. This is definitely the DRY approach towards binding implementations to interfaces. Guice did not allow me to do this - later I found the similar topic discussed in the development mailing list, where a patch is available for overriding bindings. I am yet to try it out though ..



It will be extremely helpful if some of the Guice experts address these issues and suggest workarounds. I like the terseness of the framework, the api's are indeed very intuitive and so are the error messages. The Javadocs are extremly informative, though we need more exhaustive documentation on best practices of Guice. Guice is really lightweight and is published to be very fast (I am yet to test on those benchmarks with Spring though). I hope the Google guys look into some of the pain points that early adopters have been facing with Guice ..

Wednesday, March 07, 2007

Programming Passion and the FizzBuzz Madness

Of late there have been lots of buzzing around with FizzBuzz in many of the programming blogs. It all started with Imran on Tech posing the following fizzbuzz question to interview developers who grok coding ..
Write a program that prints the numbers from 1 to 100. But for multiples of three print “Fizz” instead of the number and for the multiples of five print “Buzz”. For numbers which are multiples of both three and five print “FizzBuzz”.

The programming community took it up from there and started a deluge of self proclamation trying to establish the most efficient way of FizzBuzzing. Have a look at the comments section of all these blogs - passionate programmers have used all *official* languages to get their version of FizzBuzz going.

And the madness continues .. someone positions FizzBuzz as the the programming task of choice for discriminating hackers. And guess what ? One hacker responded with a Ruby code interpreting Prolog interpreting Lisp interpreting Lisp solving FizzBuzz.

The intent of the initial post was definitely to bring into light some of the concerns and issues that interviewers face hiring good programmers. The community responded otherwise, as Giles Bowkett aptly mentions ..
if you were absurdly cynical, you might expect programmers to start coding FizzBuzz in every language from Haskell to SQL, and even to start holding FizzBuzz coding contests.

Are programmers by nature too passionate ? I guess yes, and that is why we find such madness in the community when someone throws in some problem which has a programming smell. But is this passion justified ? Now, this is cynical, since you can never justify passion. But I think we would have been much more pragmatic focusing on the main problem that Imran on Tech had raised - the issue of hiring good programmers through a pragmatic process of interviewing. Is asking candidates to solve screening programming assignments the right way to judge a programmer in an interview ? We follow these practices rampantly, we eliminate candidates through FizzBuzz problems, yet many of the software projects fail because of bad programming and inefficient programmers. Isn't it time we step back and introspect ? Have a look at this heartfelt critique of the FizzBuzz screening question .. (reference from RaganWorld)