How much work should be done within a constructor? It seems reasonable to do some computations inside a constructor and then encapsulate results. That way, when the results are required by object methods, we’ll have them ready. Sounds like a good approach? No, it’s not. It’s a bad idea for one reason: It prevents composition of objects and makes them unextendable.
Let’s say we’re making an interface that would represent a name of a person:
interface Name {
String first();
}Pretty easy, right? Now, let’s try to implement it:
public final class EnglishName implements Name {
private final String name;
public EnglishName(final CharSequence text) {
this.parts = text.toString().split(" ", 2)[0];
}
@Override
public String first() {
return this.name;
}
}What’s wrong with this? It’s faster, right? It splits the name into parts only once and encapsulates them. Then, no matter how many times we call the first() method, it will return the same value and won’t need to do the splitting again. However, this is flawed thinking! Let me show you the right way and explain:
public final class EnglishName implements Name {
private final CharSequence text;
public EnglishName(final CharSequence txt) {
this.text = txt;
}
@Override
public String first() {
return this.text.toString().split("", 2)[0];
}
}This is the right design. I can see you smiling, so let me prove my point.
Before I start proving, though, let me ask you to read this article: Composable Decorators vs. Imperative Utility Methods. It explains the difference between a static method and composable decorators. The first snippet above is very close to an imperative utility method, even though it looks like an object. The second example is a true object.
In the first example, we are abusing the new operator and turning it into a static method, which does all calculations for us right here and now. This is what imperative programming is about. In imperative programming, we do all calculations right now and return fully ready results. In declarative programming, we are instead trying to delay calculations for as long as possible.
Let’s try to use our EnglishName class:
final Name name = new EnglishName(
new NameInPostgreSQL(/*...*/)
);
if (/* something goes wrong */) {
throw new IllegalStateException(
String.format(
"Hi, %s, we can't proceed with your application",
name.first()
)
);
}In the first line of this snippet, we are just making an instance of an object and labeling it name. We don’t want to go to the database yet and fetch the full name from there, split it into parts, and encapsulate them inside name. We just want to create an instance of an object. Such a parsing behavior would be a side effect for us and, in this case, will slow down the application. As you see, we may only need name.first() if something goes wrong and we need to construct an exception object.
My point is that having any computations done inside a constructor is a bad practice and must be avoided because they are side effects and are not requested by the object owner.
What about performance during the re-use of name, you may ask. If we make an instance of EnglishName and then call name.first() five times, we’ll end up with five calls to the String.split() method.
To solve that, we create another class, a composable decorator, which will help us solve this “re-use” problem:
public final class CachedName implements Name {
private final Name origin;
public CachedName(final Name name) {
this.origin = name;
}
@Override
@Cacheable(forever = true)
public String first() {
return this.origin.first();
}
}I’m using the Cacheable annotation from jcabi-aspects, but you can use any other caching tools available in Java (or other languages), like Guava Cache:
public final class CachedName implements Name {
private final Cache<Long, String> cache =
CacheBuilder.newBuilder().build();
private final Name origin;
public CachedName(final Name name) {
this.origin = name;
}
@Override
public String first() {
return this.cache.get(
1L,
new Callable<String>() {
@Override
public String call() {
return CachedName.this.origin.first();
}
}
);
}
}But please don’t make CachedName mutable and lazily loaded — it’s an anti-pattern, which I’ve discussed before in Objects Should Be Immutable.
This is how our code will look now:
final Name name = new CachedName(
new EnglishName(
new NameInPostgreSQL(/*...*/)
)
);It’s a very primitive example, but I hope you get the idea.
In this design, we’re basically splitting the object into two parts. The first one knows how to get the first name from the English name. The second one knows how to cache the results of this calculation in memory. And now it’s my decision, as a user of these classes, how exactly to use them. I will decide whether I need caching or not. This is what object composition is all about.
Let me reiterate that the only allowed statement inside a constructor is an assignment. If you need to put something else there, start thinking about refactoring — your class definitely needs a redesign.
| Reference: | Constructors Must Be Code-Free from our JCG partner Yegor Bugayenko at the About Programming blog. |
“But please don’t make CachedName mutable and lazily loaded — it’s an anti-pattern, which I’ve discussed before in Objects Should Be Immutable.”
Do you realize that that is probably exactly what the pre-built caching mechanisms are doing?
The point isn’t to always be immutable; it’s to be effectively immutable, to seem immutable to any outside observers. Even in pure functional languages that don’t allow mutablity, the language actually uses mutable optimizations in the background for you. That’s how TCO works.
The rest is fine, but can be considered overly pure in some situations.
this.parts = text.toString().split(” “, 2)[0];
should be
this.name= text.toString().split(” “, 2)[0];
Good article, mostly such code should be in factory method, or via some sort of build, but should say it is specific to implementation and as always it depends, String is good example – immutable why not calculate hash code in advance rather than thinking how to optimize and add complexity?