Is it a good or bad practice to call instance methods from a java constructor?
Posted
by Steve
on Stack Overflow
See other posts from Stack Overflow
or by Steve
Published on 2010-03-24T21:50:04Z
Indexed on
2010/03/24
21:53 UTC
Read the original article
Hit count: 230
There are several different ways I can initialize complex objects (with injected dependencies and required set-up of injected members), are all seem reasonable, but have various advantages and disadvantages. I'll give a concrete example:
final class MyClass {
private final Dependency dependency;
@Inject public MyClass(Dependency dependency) {
this.dependency = dependency;
dependency.addHandler(new Handler() {
@Override void handle(int foo) { MyClass.this.doSomething(foo); }
});
doSomething(0);
}
private void doSomething(int foo) { dependency.doSomethingElse(foo+1); }
}
As you can see, the constructor does 3 things, including calling an instance method. I've been told that calling instance methods from a constructor is unsafe because it circumvents the compiler's checks for uninitialized members. I.e. I could have called doSomething(0) before setting this.dependency, which would have compiled but not worked. What is the best way to refactor this?
Make
doSomethingstatic and pass in the dependency explicitly? In my actual case I have three instance methods and three member fields that all depend on one another, so this seems like a lot of extra boilerplate to make all three of these static.Move the
addHandleranddoSomethinginto an@Inject public void init()method. While use with Guice will be transparent, it requires any manual construction to be sure to callinit()or else the object won't be fully-functional if someone forgets. Also, this exposes more of the API, both of which seem like bad ideas.Wrap a nested class to keep the dependency to make sure it behaves properly without exposing additional API:
class DependencyManager { private final Dependency dependency; public DependecyManager(Dependency dependency) { ... } public doSomething(int foo) { ... } } @Inject public MyClass(Dependency dependency) { DependencyManager manager = new DependencyManager(dependency); manager.doSomething(0); }This pulls instance methods out of all constructors, but generates an extra layer of classes, and when I already had inner and anonymous classes (e.g. that handler) it can become confusing - when I tried this I was told to move theDependencyManagerto a separate file, which is also distasteful because it's now multiple files to do a single thing.
So what is the preferred way to deal with this sort of situation?
© Stack Overflow or respective owner