Structuring a class

Question

I have a couple methods that need to run sequentially. They all live in the same class. Is it better to structure them like this?

public class Test{
 public Test(){
 foo1();
 }
 private void foo1(){
 //do stuff
 foo2();
 }
 private void foo2(){
 //do stuff
 foo3();
 }
 private void foo3(){
 //do stuff
 }
}

Or this.

public class Test{
 public Test(){
 doFoo();
 }
 private void doFoo(){
 foo1();
 foo2();
 foo3();
 }
 private void foo1(){
 //do stuff
 }
 private void foo2(){
 //do stuff
 }
 private void foo3(){
 //do stuff
 }
}

Which way would be more clear? I'm leaning towards the second example, but i'm not sure if having a method that only calls other methods is best practice.

• Kayaman: The first way is more unclear, since you're doing daisy-chaining.
• user784540: Why do you make an additional method to invoke foo1, foo2 and foo3 ? Invoke them directly in your constructor.
• Ryan: I prefer the second example as well. It is easier to follow the flow.
• Manu: I'm not sure if having a method that only calls other methods is best practice. Why? If you were writing the code without thinking about this, you would first write the 3 pieces of code inside the method, then realise they do independent stuff, thus you extract them into 3 private methods.
• phflack: Are these methods being reused independently at all? It may be a good idea to keep them separate, unless foo2 and foo3 are always called together in which case you may as well just make it foo23

Answer 1

I think it depends on the operations. If operation foo2 is part of what foo1 does, then you should call foo2 from foo1.

If foo1 and foo2 are unrelated and only part of the foo operation, then call these two from foo.

It depends on the semantics of the methods which call hierarchy is clearer and therefore easier to understand.

Answer 2

The second example is better for multiple reasons.

First, you can write unit tests to test foo1, foo2 and foo3. With the first method you can only do a unit test for foo1.

Second, the second solution is better if in the future you have to change the order of your calls.

• Manu: I don't understand your argument about testing. Why would you only be able to test foo1 in the first case?
• Magus: @Manu You can make a test for foo2, but foo2 is already tested with foo1. And in fact, if foo3 crash, then foo1 and foo2 will crash too. So that's not that you can't test foo2 and foo3, but results may be hard to deal with.
• Dave Newton: The correct approach for testing each method is to stub/mock the nested methods. That said, there's nothing fundamentally wrong with a unit test at the foo1 level that indirectly tests foo2 and foo3, although it's a hybrid unit/integration test. It depends on method visibility and complexity at the very least.

Answer 3

I can see both of the methods have their pros and cons. As far as readability goes the second example is much easier to follow. There is one example where the first one might be useful though.

public class Test{
 public Test(){
 foo1();
 }
 private int foo1(){
 //do stuff
 if(something went wrong) return -1;
 foo2();
 return 0;
 }
 private int foo2(){
 //do stuff
 if(something went wrong) return -1;
 foo3();
 return 0;
 }
 private int foo3(){
 //do stuff
 if(something went wrong) return -1;
 return 0;
 } 
}

Depending on wheter you want to execute foo3 regardless whether or not an error occurs the second example would be better. If you only want to execute foo3 if foo2 is successful the first method might be useful.

• Manu: You should add return in front of foo2() and foo3()

Answer 4

That completely depends on what these methods do.

If they are really three steps that need to be done in sequence then version 2 is better.

But if they are "nested" steps where foo2 is part of what needs to be done in foo1 then version 1 is better.

Example for the second case: writeJavaBeanToFile transforms the bean to a Map, then calls writeJavaMapToFile which transforms the Map to a String and calls writeStringToFile.

Answer 5

You should follow second method which is also a pattern - facade pattern. You basically want to do 3 operations in sequence and your invocation is made much simpler if you map these 3 to 1 method. This is what facade pattern also does - easing up the invocation interface.