I was recently thrown into a Java web application project, and I’ve come across a number of classes that follow this type of format:
public class MyThingy {
private final int p1;
private final String p2;
…
public MyThingy (int p1, String p2, …) {
this.p1 = p1;
this.p2 = p2;
…
}
public static void doSomething(int p1, String p2, …) throws Throwable {
final MyThingy myThingy = new MyThingy(p1, p2, …);
myThingy.execute();
}
private void execute() throws Throwable {
//do stuff
}
}
It seems like this could be accomplished with the following code, which to me seems way easier to read.
public class MyThingy {
public static void doSomething (int p1, String p2, …) throws Throwable {
//do stuff
}
}
The only possible benefit I can see from doing it the first way, is that if you had to break up execute() into smaller pieces, they could all share the initial parameters without having to explicitly pass them around. But this maybe only benefits the lazy coder, as it becomes difficult for the reader to tell which methods need which parameters and when the values might be changed (akin to global variables.)
Is there something I'm missing? Threading, performance?
Edit: I should have mentioned, although the constructor is public, it is not called. The only usage is like this:
MyThingy.doSomething(p1, p2...);
Aside from this in itself being problematic for testing, I can't see any reason not to put the logic of execute() directly into doSomething(). Even if we were to get rid of the static function, the constructor still doesn't make sense to me; I think the parameters should be passed directly to the method that will use them.
throws Throwable
is a bad practice, it should be at leastthrows Exception
, or something more specific, if possible. And, since bad practices usually come together, I'd say that this code template is just another bad practice.new(...)
+execute()
in one call.final
). The static method is the only API into the object since all members are private. In just the code you've shared, I don't see any benefits to this over makingexecute
a static method taking p1, p2, ....