views:

186

answers:

7

I'm working with some existing code, trying to add to it and increase the unit tests for it. But running into some problems with getting the code testable.

Original Constructor:

public Info() throws Exception
{
  _ServiceProperties = new ServiceProperties();
  _SshProperties = new SshProperties();
}

I'm aware that this is bad, and obviously not testable. In a junit environment, this class will fail to create every time since it wont be able to find the necessary properties to construct itself. Now, I'm aware this class would be a lot more testable with the simple change of moving anything prefaced with 'new' as a parameter.

So I end up with:

New Constructor:

public Info(ServiceProperties srvProps, SshProperties sshProps) throws Exception
{
  _ServiceProperties = srvProps;
  _SshProperties = sshProps;
}

Which allows me to properly unit test this Info class. The problem though, is now all that work is pushed to some other class:

Some Other Class' Method:

public void useInfo() throws Exception
{
  ServiceProperties srvProps = new ServiceProperties();
  SshProperties sshProps = new SshProperties();
  Info info = new Info(srvProprs, sshProprs);
  doStuffWithInfo(info);
}

Now this method isn't testable. All I've managed to do is push off where the constructions of these Property objects are occurring, and somewhere else some piece of code is going to be stuck actually having to call "new".

Here's the rub for me: I can't figure out how to break this chain of events of simply pushing these "new" calls somewhere else. What am I missing?

+7  A: 

Look at using a Dependency Injection framework such as Spring. This application of Inversion of Control means that each of your types can avoid the pitfall you've seen, leaving the configuration to "wire" components together.

This introduction to Spring (pdf) gives a comprehensive overview of Spring. The first chapter or two should be sufficient to explain the concepts.

Also see Inversion of Control Containers and the Dependency Injection pattern by Martin Fowler

Rich Seller
You can also take a look at our Spring course material: http://www.trainologic.org/courses/info/2
Shimi Bandiel
A: 

The easiest answer is Spring. However another answer is to put your config stuff into JNDI. Spring in some ways is a better answer, especially if you don't have anything that changes depending on environment.

Jim Barrows
+1  A: 

You have the right idea. Perhaps this will help you. I recommend you follow two rules for all your classes of significance, where "of significance" means if you don't follow the steps it will be more difficult to test, reuse, or maintain the class. Here are the rules:

  1. never instantiate or self-acquire a dependency
  2. always program to interfaces

You have a start at rule #1. You changed your Info class to no longer create its dependencies. By "dependency" I mean other classes, configuration data loaded from property files or whatever, etc. When you depend on how something is instantiated you are tying your class to it and making it more difficult to test, reuse and maintain. So, even if a dependency is created via a factory or a singleton, don't have your class create it. Have something else call create() or getInstance() or whatever and pass it in.

So you chose the "something else" to be the class that uses your class, and realized there is a bad smell to it. The remedy is to instead have the entry-point to your application instantiate all dependencies. In a traditional java app, this is your main() method. if you think about it, instantiating classes and hooking them up to each other, or "wiring" them together, is a special kind of logic: "application assembly" logic. Is it better to spread this logic throughout your code, or to collect it in one place to more easily maintain it? The answer is that collecting it in one place is better - not only for maintainance, but the act of doing so turns all your classes of significance into more useful and flexible components.

In your main() or equivalent of main() you should create all the objects you need, passing them into each others' setters and constructors to "wire" them together. Your unit tests would then wire them differently, passing in mock objects or similar things. The act of doing all this is called "dependency injection". After doing as I say, you will likely have a big ugly main() method. This is where a dependency injection tool can help you out and in fact make your code infinitely more flexible. The tool I would suggest when you get to this point, as others have also suggested, is Spring.

The less important rule #2 - always program to interfaces, is still very important because it eliminates all dependencies on implementation, making reuse much easier, not to mention leveraging other tools like mock object frameworks, ORM frameworks, etc. easier as well.

SingleShot
+1  A: 

Even dependency injection frameworks like Spring, Guice, PicoContainer etc. need some sort of boostrap so you always have to build something up.

I would suggest you to use a provider/factory that returns a configured instance of you class. This would allow you to exit the "creation"-hierarchy.

MrWhite
A: 

You let the some-other-class have too much knowledge about the Info class and its dependencies. A dependency injection framework would use a provider class. Using generic types one can make a Provider of Info objects:

interface Provider<T> {
  T get();
}

If your some-other-class take a Provider<Info> in its constructor your method would look like:

public void useInfo() throws Exception
{
  Info info = infoProvider.get();
  doStuffWithInfo(info);
}

This has removed construction of concrete classes from your code. Next step is to make Info into an interface to make it easier to create a mock for the specific unit-test case.

And yes, this will push and push all the object construction code further and further up. It will lead to some module that only describes how to wire things together. The "rules" for such code is that it should be free of conditional statements and loops.

I recommend reading Misko Heverys blog. All the presentations are useful and printing out the guide to writing testable code as little rulebook once you understand the rules is a good thing.

Christian
+1  A: 

Your constructors aren't incorrect and the problem isn't about when/where code is executed, it's about what everyone else mentioned: Dependency Injection. You need to create mock SshProperties objects to instantiate your object. The simplest way (assuming the class isn't marked as final) is to extend the class:

public class MockSshProperties extends SshProperties {
    // implemented methods
}

You can you use mock frameworks like Mockito:

public class Info {

    private final sshProps;
    private final serviceProps;

    public Info() {
        this(new SshProperties(), new ServiceProperties());
    }

    public Info(SshProperties arg1, ServiceProperties arg2) {
        this.sshProps = arg1;
        this.serviceProps = arg2
    }
}

public class InfoTester
{
    private final static SshProperties sshProps = mock(SshProperties.class);
    private final static ServiceProperties serviceProps = mock(ServiceProperties.class);
    static {
        when(sshProps.someGetMethod("something")).thenReturn("value");
    }

    public static void main(String[] args) {
        Info info = new Info(sshProps, serviceProps);
        //do stuff
    }
}
Droo
+1  A: 

take a look at this video. It gives some nice insight in this topic: http://www.youtube.com/watch?v=RlfLCWKxHJ0&amp;feature=youtube%5Fgdata

Juri
Related reading:http://misko.hevery.com/http://misko.hevery.com/category/testability/http://googletesting.blogspot.com/search/label/Misko
VoiceOfUnreason
Thanks for the material. Google Tech vids are usually always a good source for best practices. There's a lot of nice material.
Juri