Oleksandr
Oleksandr

Reputation: 2416

Refactoring in order to write "pretty" JUnit tests

I want to clarify refactoring in scope of TDD.

Before:

class Somclass{
      public void sendMessage(){    
       WebServiceStub stub = new WebServiceStub();     
      ...
      stub.sendMsg();        
      }
    }

After:

class Somclass{
private WebServiceStub stub;

  public void sendMessage(){
    ...
  if(stub == null){
   stub = new WebServiceStub();
  }
  ...
  stub.sendMsg();        
  }
}

So I want to verify sendMsg() method and make some asserts with result. To have posibility to mock this stub i move this stub local variable to instance variable. So that I can set mocked stub to class and do verivyings and asserts in test class. For example:

@Test
public void testSMth(){
  wsProvider.setStub(stubMock);
  verify(stubMock).sendMsg();
  ...asserts
}

This approach is not thread safety and I should do some concurrency modification. This modification may cause mistakes. So in local variable approce there is thread safty.

Also i could create Factory that will return an instance of WebServiceStub. But this approache will produce new classes because this situation is frequent.

There is a question: how test this cases and does goot test cost modification that may cause mistakes?

Upvotes: 3

Views: 853

Answers (3)

Sean Reilly
Sean Reilly

Reputation: 21836

Use constructor injection to avoid the possibility that the dependency is not set. This will allow you to easily use a mock in your tests.

If the WebServiceStub class is in fact not thread-safe (but if WebServiceStub is generated by JAX-WS, then you should know that metro/jax-ws stubs often are thread safe), then yes, you will have to use a factory. This isn't really a big deal, and it shouldn't slow you down that much. You can use a static inner class if you want.

Upvotes: 3

Morten
Morten

Reputation: 3844

It looks almost right, but never instantiate stub, if stub == Null. Instead, thow ArgumentNullException. Null should never accepted as a valid argument (unless you have a really, really, really good reason).

Upvotes: 0

artbristol
artbristol

Reputation: 32407

Your class should have the WebService object (I refuse to call it a 'stub') as a field.

class Someclass{

  @Resource
  private WebService ws;

  public void sendMessage(){

  ws.sendMsg();        
  }
}

It should be injected with a DI framework of your choice. In your test, you can set it to a mock. There's no need for a lazy getter, as your point out that's not thread-safe.

Upvotes: 4

Related Questions