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: 195
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
doSomething
static 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
addHandler
anddoSomething
into 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 theDependencyManager
to 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