views:

86

answers:

5

So I have a helper method that looks something like the following:

private D GetInstanceOfD(string param1, int param2)
{
    A a = new A();
    B a = new B();
    C c = new C(a,b, param1);

    return new D(c, param2);
}

It's just a convenient helper method for which I can call to grab a particular object that I need rather then to remember which dependencies I need to hook up to get the object I require.

My first question here is: should methods like these be tested? The only reason I can think of to want to test these type of methods would be to make sure that the correct dependencies are used and are set up correctly.

If the answer to the first question is yes, my second is: how? I am currently using NUnit and RhinoMocks and am trying to work out how this method has to be refactored to be testable (well, and whether something like this should be tested!); dependency injection obviously would not work here as this method actually creates the dependencies!

Or is using this method bad practice and I should be doing something like the following:

D d = new (new C(new A(), new B(), "string"), 1024);
+3  A: 

To answer your first question, yes you should test this method. Testing this method will take a lot less effort than writing this entire post and will give you validation on shipping code.

In terms of how that depends on what you intend the method to do. Here are the following things you could test assuming reasonable behavior of the function

  • What happens if I pass null for the first param?
  • What happens if I pass a negative number for the second param?
  • Does GetInstanceOfD actually return a D in the cases you'd expect?
  • Does D have the correct values for the given input.
  • What about the C, A and B instance assuming they're accessible?
JaredPar
Generally, my private methods come about as a result of refactoring to keep the code DRY or methods small. In that case, the code actually is probably already tested by existing tests before the refactoring. In that case I wouldn't normally write tests for this method directly, but rely on the existing tests and any new tests for public methods. That's not always the case and I do have some tests for private methods that I test using reflection/accessors, but it's typically the case that I only test the public interface so my tests aren't unnecessarily coupled to the internals of the class.
tvanfosson
+2  A: 

When you run into questions like this, one good thing to keep in mind is to keep things trivial.

Setup methods need to be trivial. Teardown methods need to be trivial. Helper methods can be nontrivial -- if they have their own sets of tests.

If your unit tests themselves are becoming so complex the need to test them arises, you're doing it wrong.

Jon Limjap
+1  A: 

First, the method is private and, so, normally I would say that testing is probably not necessary for that method directly. Even if it were a helper method for real code, instead of a helper for your tests, you may want to only test it indirectly through the public interface.

Second, if you do want to test it -- and perhaps anyway -- you should consider using factories to do the object creation rather than directly instantiating the objects. Using a factory, in conjunction with interfaces, will allow you to abstract the dependencies out. Simply inject the factory into the constructor of the class containing this method and use it as a property within this method to create the proper objects. If you go this route, you may also find that the factory is also the place for the logic to create D.

This would definitely be the direction I would take if the code were production code. Example below:

public class ObjectFactory 
{
     public virtual A CreateA() { return new A(); }
     public virtual B CreateB() { return new B(); }
     public virtual C CreateC( string str ) { return new C( CreateA(), CreateB(), str );
     public virtual D CreateD( string str, int num )
     {
         return new D( CreateC( str ), num );
     }
}


public class Foo
{
    private ObjectFactory ObjectFactory { get; set; }

    public Foo() : this(null) { }
    public Foo( ObjectFactory factory )
    {
         this.ObjectFactory = factory ?? new ObjectFactory();
    }

    public void Bar()
    {
        D objD = this.ObjectFactory.CreateD( param1, param2 );
    }
}

[TestMethod]
public void TestBar()
{
     var factory = MockRepository.GenerateMock<ObjectFactory>();
     var d = MockRepository.GenerateMock<D>();
     factory.Expect( f => f.CreateD( "abc", 10 ) ).Return( d );

     var foo = new Foo( factory );

     foo.Bar();

     factory.VerifyAllExpectations();
     d.VerifyAllExpectations();
}
tvanfosson
A: 

private methods are usually an indicator that they don't need to be directly tested because you will be testing methods that use these private methods.

That method feels like it belongs in factory or builder class, not as a private method. It won't be elegantly testable if you keep instantiation code hidden as a private method.

aberrant80
+1  A: 

You should test this method. You should test it as it can fail; one XP adage is Test everything that could possibly break.

Now, this does not mean that you have to create new tests for it; it can be indirectly tested, thru other methods. If it has been extracted during a refactoring, then it probably is already tested.

One way to test it, should the need arise, would be to extract it into a helper class, where it won't be private, and to split it into several functions to be able to inject dependencies.

//--- don't write this, you'll probably regret it ;o)
D d = new (new C(new A(), new B(), "string"), 1024);
philippe