I personally find that this code snippet demonstrating constructor chaining is hard to read.
This code makes it very hard to identify which of the many constructors is the "real" one: the constructor that does the real job and that I should call if I add a new overloaded constructor.
This is why I use the following two rules when I write overloaded constructors:
- Have all your constructors converge toward one unique private method that you will name consistently throughout your code base (e.g. init) and that will contain the entirety of all the parameters that your constructors may receive.
- Don’t add any logic to the constructors, all they should do is invoke init with the right parameters and null for default ones.
Here is an example, from worst:
public class Person { private String m_firstName = null; private String m_lastName = null; private long m_ssn = 0; public Person(String lastName) { m_firstName = null; m_lastName = lastName; m_ssn = getSsn(); } public Person(String firstName, String lastName) { m_firstName = firstName; m_lastName = lastName; }
…
to better:
public Person(String lastName) { this(null, lastName); m_ssn = getSsn(); } public Person(String firstName, String lastName) { m_firstName = firstName; m_lastName = lastName; }
… to best:
public Person(String lastName) { init(null, lastName); } public Person(String firstName, String lastName) { init(firstName, lastName); } private void init(String firstName, String lastName) { m_firstName = null; m_lastName = lastName; if (null == m_firstName) { m_ssn = getSsn(); } }
Here is why the latter form is preferable:
- It doesn’t duplicate any code.
- It makes the initialization rules clear : "if no firstName was given, then ssn gets initialized".
- The signature of init gives a good indication of the various parameters this class needs to be initialized, while a multitude of overloaded constructors obscures this fact.
#1 by Luc Claes on November 5, 2004 - 10:30 am
Declaring members ‘final’ is a useful way to force initialization at construction time:
private final String m_lastName;
public Person(String lastName) {
m_lastName = lastName;
}
Doesn’t the ‘init()’ construct prevent such a declaration ?
#2 by Pierre CARION on November 5, 2004 - 10:44 am
As Luc, I loves final member so that I know I don’t need to worry about them later on 😉
the init() pattern does not allow that.
#3 by Anonymous on November 5, 2004 - 10:49 am
The problem I have this is that there is a secret Object() constructor called before each of the init() calls in the 3rd example. Now, this may be fine for classes which subclass Object. What about when you have superclasses which don’t define a no-op constructor?
I despise the hidden constructor calls, and always make the call explicit (ie, add a super(); before the init() calls). I also despise hidden no-arg constructors, and always add one explicitly. Too much magic.
#4 by Cedric on November 5, 2004 - 11:05 am
Yes it does!
Here is what I responded to Luc:
> Doesn’t the ‘init()’ construct prevent such a declaration ?
No, if you don’t initialize your field in the declaration, you can assign it later:
private final String m_firstName;
private void init() {
m_firstName = …
}
Making them final guarantees they are only initialized once, but you can do it anywhere.
Besides, there are times where you can’t initialize your class at construction time, but that’s a different debate.
—
Cedric
#5 by Tim Haley on November 5, 2004 - 12:00 pm
Luc is correct…
public class Person
{
private final String m_firstName;
private final String m_lastName;
private final String m_ssn;
public Person(String lastName)
{
init(null, lastName);
}
public Person(String firstName, String lastName)
{
init(firstName, lastName);
}
private void init(String firstName, String lastName)
{
m_firstName = firstName;
m_lastName = lastName;
if (null == m_firstName)
{
m_ssn = getSsn();
}
}
private String getSsn()
{
throw new UnsupportedOperationException(); //todo
}
}
Compile your suggested code and you will get the following errors:
Information: 3 errors
Information: 0 warnings
Information: Compilation completed with 3 errors and 0 warnings
C:\IDEAProjects\IBEAM\src\wm\Person.java
Error: line (20) cannot assign a value to final variable m_firstName
Error: line (21) cannot assign a value to final variable m_lastName
Error: line (24) cannot assign a value to final variable m_ssn
#6 by Tim Haley on November 5, 2004 - 12:13 pm
BTW, the original source that you referenced, does support final members and almost satisfies your requirements:
Have all your constructors converge toward one unique … method … and that will contain the entirety of all the parameters that your constructors may receive. (it’s the constructor with the most parameters)
Don’t add any logic to the constructors, all they should do is invoke [the common constructor] with the right parameters and null for default ones.
#7 by Luc Claes on November 5, 2004 - 12:41 pm
I’m not sure I’m totally correct 🙂
By experience, constructs like:
public class FinalTest {
private final String m_strLastName;
public FinalTest(String strLastName) {
init(strLastName);
}
private void init(String strLastName) {
m_strLastName = strLastName;
}
}
… are rejected by my (Eclipse) compiler.
But is Eclipse right or too conservative ?
The Java specification concerning ‘blank finals’ initialization is rather complex (http://java.sun.com/docs/books/jls/second_edition/html/defAssign.doc.html)
#8 by Cedric on November 5, 2004 - 12:45 pm
I stand corrected, it’s not possible to assign a final value outside of the constructor… serves me right for not trying it.
Tim is right, if you have final values, you need to pick one constructor to be the canonical one, but if you have no final variables, I think init() is easier to locate when you browse your source.
#9 by Jonathan Ellis on November 5, 2004 - 1:04 pm
Perhaps not central to your main point, but using a null firstname to indicate “init ssn” is poor style as well…
#10 by a on November 5, 2004 - 1:09 pm
How is reading the name of a constructor easier than reading the name of a method? It’s not hard to scroll down until you see a bunch of code. If you are going to create a non-compiler-enforced convention, why not make it one that specifies the “doit” constructor is the first thing declared in the class?
Of course, all this wouldn’t be necessary if you could specificy default values a la C++
#11 by a on November 5, 2004 - 1:09 pm
How is reading the name of a constructor harder than reading the name of a method? It’s not hard to scroll down until you see a bunch of code. If you are going to create a non-compiler-enforced convention, why not make it one that specifies the “doit” constructor is the first thing declared in the class?
Of course, all this wouldn’t be necessary if you could specificy default values a la C++
#12 by a on November 5, 2004 - 1:10 pm
sorry for double post, caught a logic-reversal bug after I hit submit. Second snide comment is the intended one 🙂
#13 by Cedric on November 5, 2004 - 1:13 pm
Because the name of the constructor changes for every class, while init() can be kept constant throughout your code base.
#14 by Rob Meyer on November 5, 2004 - 3:03 pm
Seems to me that a standardized comment (“this is the init constructor”) give you the same scannability for the “Real” constructor. Maybe even a custom javadoc tag to identify the “full constructor” in the api-docs? @fullconstructor or something…
#15 by Ryan LeCompte on November 5, 2004 - 3:50 pm
I don’t think that having an init() method is always a good idea, since this method may unintentionally be called in places OTHER than the constructor. Initialization code should be put in the constructor. If there is common code that is used when the class is first initialized as well as later on (ie, it has the notion of being “reinitialized”) then having a “reInitialize()” type of method this is called from the constructor (and potentially other places) is desirable.
#16 by Keith Lea on November 5, 2004 - 4:31 pm
You could also create an annotation so you could declare
private @Initializer MyClass(lots of, args here) {..}
#17 by Nick Minutello on November 5, 2004 - 5:20 pm
Hmmm
#18 by Marc Logemann on November 6, 2004 - 12:25 am
Ok, i will join this one, because i was quoted as bad example 😉
First of all, i like init() methods and i use them all over the time, but they have some risks. Its easy to call them more than once when you do inheritance.
public class Person extends Human {
public Person() {
super();
init();
}
private void init() {
//init stuff here
}
}
public class Human {
public Human () {
init()
}
private void init() {
//init stuff there
}
}
What i mean is, you can easily come into trouble when you dont chain the init() methods correctly. And i think that my example is really not hard to read, you have a lot of this-constructors and one with some code. But technically these two ways are not too far away, both try to focus on a certain place where all the stuff happens.
Marc
#19 by Marc Logemann on November 6, 2004 - 12:47 am
Note: to prevent comments like “why do you use super() in Person()” –> i just wanted to outline the callgraph, for the case that perhaps 2% of the visitors dont know about the auto-insertion of super() 🙂
#20 by Julian on November 7, 2004 - 11:27 am
Whenever I have more than one constructor, one of them is called by all the others, as in the JTextField example you linked to. That approach is clear and avoids redundancy.
Besides, my classes rarely have more than 2 or 3 constructors, keeping them from being that confusing.
#21 by MmeGuerin on November 8, 2004 - 11:46 am
When I design a class, I always try to apply the following rule : Constructors must not invoke overridable methods, directly or…indirectly !
Having said that, since Person class is not final, I’m assuming the getSsn() method in your example should not be overridden ?!
Invoking overridable methods from a constructor might cause errors because the object is not in a a valid state.
#22 by B. K. Oxley (binkley) on November 8, 2004 - 5:08 pm
Cedric, you are right that I did elide your point of finding the privileged constructor with a special name. Would you find my counter-example if the “real” constructor was listed first and all the others following? I am a strong believer the value of clarity, so I take your point quite seriously.
#23 by Oliver Kamps on November 9, 2004 - 1:02 am
I’m with what seems to be the majority of the crowd and would prefer to call a chained constructor.
I agree with you that the body of this constructor should be as expressive as your init method, i.e. it should communicate clearly under which circumstances we need to initialize the SSN.
I’d also avoid long chains of constructor calls, i.e. calling a single other constructor should suffice in almost all cases.
Cheers,
Oliver
#24 by Mike Lehmann on November 9, 2004 - 6:00 am
The best is to make the variables final (which is impossible using the init-method):
public class Person {
private final String m_firstName;
private final String m_lastName;
private long m_ssn = 0;
public Person(String lastName) {
this(null, lastName);
m_ssn = getSsn();
}
public Person(String firstName, String lastName) {
m_firstName = firstName;
m_lastName = lastName;
}
}
#25 by Joe Maia on November 10, 2004 - 11:25 pm
It is standard practice to have one constructor – the one with the most parameters – do ALL the work, with all other constructors calling that one constructor. Since this is the *standard*, it is also the most readable because most programmers reading your code will *expect* this pattern.
I find it is rarely the case that values I initialize in constructors can be declared as “final”. Sometimes this may be the case, but many times the values might change over the course of time.
#26 by Anonymous on November 10, 2004 - 11:31 pm
It is standard practice to have one constructor – the one with the most parameters – do ALL the work, with all other constructors calling that one constructor. Since this is the *standard*, it is also the most readable because most programmers reading your code will *expect* this pattern.
I find it is rarely the case that values I initialize in constructors can be declared as “final”. Sometimes this may be the case, but many times the values might change over the course of time.
#27 by sarsor on February 19, 2005 - 5:34 am
Why chaining constructors is bad.
Ping Back???blog.csdn.net