3

I have a service that does fairly complicated business logic when it receives a request. I have most the functionality isolated into private methods which dive a couple levels down from the method that receives the request. Basically receive request -> private method for subprocessing -> private method for processing different action -> method that creates a result -> method that verifies result.

I can either call each of these methods sequentially and pass around the various results between each private method or I can call one method from inside of the next method and keep passing along the data between steps the other way

//either passing around as returns or class variables
public onRequest(Request request){
        method1(request) // Returns two things for method 2
        method2(thing1, thing2) //Modifies and thing1 and thing2
        method3(thing1Mod, thing2Mod) //Returns a result
        verify(result)
        return result
}

Option 2

public onRequest(Request request{
    Result result = method1(request)
}

method1(request){
    //logic that makes thing1 thing2
    return method2(thing1, thing2)
}

method2 (thing1, thing2)
    //logic to modify thing1 and thing2
    return method3(thing1, thing2)

method3 (thing1, thing2) {
    //logic that gets a result from thing1, thing2
    verify(result)
    return result
}

Each method contains discrete functionality.

The reason I am concerned is that I am trying to write test cases and I want to only leave onRequest public but I still wanna test methods 1-3 and verify individually.

Project is in Java

  • I am using PowerMock and Mockito so I can mock the input or output of a given method and prevent it from being called – ford prefect Oct 26 '15 at 18:18
  • I can understand - sort of - why you would call method2 from method1, but why is verify(result) in method3? Wouldn't it be better if the verification of the returned result was as close to the request as possible, ie in OnRequest, after the call to Method1 that creates it? – Marjan Venema Oct 26 '15 at 19:38
  • Oh and by the way, provided the method names in your actual code make it obvious what is going on, I would prefer option 1 if the steps are fairly independent and may even be optional. This way the recipe for returning and verifying a result is easily understood from OnRequest. Much more so than in option 2, where much of the logic is hidden "downward". – Marjan Venema Oct 26 '15 at 19:41
2

Its the wrong question

Of course you should "pass around a result." Not only the points @Ewan makes about testing, but passing parameters makes code more maintainable generally. And as a bonus you can say "I'm using dependency injection." ooohhh.

The above says nothing about the code structure!

... or I can call one method from inside of the next method and keep passing along the data between steps the other way

You need to decide which steps - the afore mentioned methods - are co-equal, sequential steps of a given process and which truly are sub-steps of their containing method. This gives your program a foundational structure. It essentially defines the processing layers, and I promise you, has an effect on how the program is understood, hence where change should be made, and how the code evolves in the long run.

Encapsulation and Testability

Ideally a layer (one method or a hierarchy of methods) will not expose its guts to the outside, and parameters helps in this regard.

As for testability, sometimes my test class will inherit the class I'm testing and then I can test protected methods. Which means I rarely make methods private just for that reason.

And sometimes it is ok to simply infer the correctness of inner methods - select test parameter values carefully.

And finally, I'll be honest here - I've made inner methods public for testing purposes - because code structure/design comes first.

3

If you want to unit test the otherwise private methods you have a number of choices

  • Reflection ! yay reflection.

  • Make them protected and expose by creating an accessor class which inherits from the original

  • Make the methods public, but put them on other sub-service classes, which your original class then has private instances of.

Once you have exposed them in one of these ways, I think you can see that your first option, that of calling each in turn from the main method, gives you the best 'testablity' as you are able to test each method separately from the others. Whereas your second option, makes it difficult to test method1 if method2 is failing for example.

  • +1 My vote goes to making them protected and using a test descendant (your accessor class). It is only a small tweak for testability in the production code as the test descendant itself would only have to live in the test project. (Yes, reflection wouldn't require any tweaks, but I like to keep my test code as independent from reflection as possible). – Marjan Venema Oct 26 '15 at 19:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.