Is there ever a reason to do all an object's work in a constructor?
- by Kane
Let me preface this by saying this is not my code nor my coworkers' code. Years ago when our company was smaller, we had some projects we needed done that we did not have the capacity for, so they were outsourced. Now, I have nothing against outsourcing or contractors in general, but the codebase they produced is a mass of WTFs. That being said, it does (mostly) work, so I suppose it's in the top 10% of outsourced projects I've seen.
As our company has grown, we've tried to take more of our development in house. This particular project landed in my lap so I've been going over it, cleaning it up, adding tests, etc etc.
There's one pattern I see repeated a lot and it seems so mindblowingly awful that I wondered if maybe there is a reason and I just don't see it. The pattern is an object with no public methods or members, just a public constructor that does all the work of the object.
For example, (the code is in Java, if that matters, but I hope this to be a more general question):
public class Foo {
private int bar;
private String baz;
public Foo(File f) {
execute(f);
}
private void execute(File f) {
// FTP the file to some hardcoded location,
// or parse the file and commit to the database, or whatever
}
}
If you're wondering, this type of code is often called in the following manner:
for(File f : someListOfFiles) {
new Foo(f);
}
Now, I was taught long ago that instantiated objects in a loop is generally a bad idea, and that constructors should do a minimum of work. Looking at this code it looks like it would be better to drop the constructor and make execute a public static method.
I did ask the contractor why it was done this way, and the response I got was "We can change it if you want". Which was not really helpful.
Anyway, is there ever a reason to do something like this, in any programming language, or is this just another submission to the Daily WTF?
