What are the pro's and con's of calling out to a mutator from a constructor (if any)
i.e.:
public MyConstructor(int x) {
this.x = x;
}
versus:
public MyConstructor(int x) {
setX(x);
}
public void setX(int x) {
this.x = x;
}
Do you have a preference? (This is not homework, just looking at our coding standards doc where it says to always call out to mutators when setting instance var's in the constructor and I don't always to this)
Personally, I would set the variable directly in most cases.
Methods usually expect that the instance is fully-formed by the time they're called. In particular, calling an overridden method from a constructor is a recipe for hard-to-understand code and hard-to-spot bugs.
Having said that, I often try to make classes immutable anyway, in which case not only is there no setter, but you have to set the final variable from the constructor (or a variable initializer) anyway :)
Where properties have logic, setter logic is usually validation and sometimes change propagation to observers. I'd usually expect the constructor parameters to be checked explicitly at the start of the method, and you wouldn't want any change propagation to occur before an instance is fully created anyway.
I follow two rules about constructors to minimize problems which are why I would not use the mutator method:
Constructors (of non-final classes) should call only final or private methods. If you decide to ignore this rule and let the constructor call non-final/non-private methods, then:
Is all that extra cognitive baggage worth it? You could allow an exception for simple mutators that only assign a value to an instance variable, since there's little benefit, even that doesn't seem worth it.
[[ @Jon Skeet mentions this in his answer: "... In particular, calling an overridden method from a constructor is a recipe for hard-to-understand code and hard-to-spot bugs." But I don't think the ramifications of this problem is stressed enough. ]]
Constructors should be cautious about leaking this before the instance is fully initialized. While the previous rule was about methods inside the class and subclasses accessing ivars, you must also be careful about (even final/private) methods passing this to other classes and utility functions before this is fully initialized. The more non-private, overridable methods that the constructor calls, the greater the risk of leaking this.
Some references about constructors calling non-final, non-private methods:
Invoking any public, static, non-final methods within constructor it's up to you, but the best practice is never invoke such methods within constructor, because this methods can be overridden in subclasses and actually only overridden version of this methods will be invoked (If you use polymorphic behavior).
For example:
public class Foo {
public Foo() {
doSmth(); // If you use polymorphic behavior this method will never be invoked
}
public void doSmth() {
System.out.println("doSmth in super class");
}
public static void main(String[] args) {
new Bar(200);
}
}
class Bar extends Foo {
private int y;;
public Bar(int y) {
this.y = y;
}
@Override
public void doSmth() { // This version will be invoked even before Barr object initialized
System.out.println(y);
}
}
It will print 0.
For mo details read Bruce Eckel "Thinking in Java" chapter "Polymorphism"
My preference is to set them directly in the constructor for a few reasons. Firstly something like this.x = x; is just as clear, if not more so than calling a separate method that does the same thing. Secondly the method may have potentially been overridden unless it's marked final, and calling potentially overridden methods from a constructor is a big no-no in my book. Thirdly, most methods generally assume that the object is already complete when they're executing, not half way through being constructed. Whilst this shouldn't cause any issues in this simple case, in more complex cases it can cause seriously subtle bugs that take ages to track down.
The main argument for using setters / getters everywhere is that it means you can rename the field by just changing its name in 3 places, its definition, the getter / setter methods, and all should compile and be fine. That argument is null and void these days in my opinion, since any decent modern IDE will rename all occurrences of such a field with a simple keyboard shortcut.
Unless your setter is doing something more complex than this.x = x, I would just set the variable directly. The parameter maps directly to your instance variable, and to me it's more intent-revealing.
I rarely do it, and I've never had an issue because of that. It can also have unintended consequences, especially if your setters are overridden in a subclass, or if your setters fire off additional method calls which might not be appropriate during initialization.
There is a single reason why our coding standards require the use of accessors (which may also be private). If you quickly want to find out what code is changing a field, you simply bring up the call-hierarchy for the setter in Eclipse!
This is a huge time-saver in a codebase that has reached 2 million lines of code and also eases refactoring.
It depends on how you treat setters. If you believe that the class can be derived from, you might allow such a call out so that you can override the behavior elsewhere. Otherwise, you can set it as it.
Of course, Allowing setters explicitly be called would also provide ways to inject additional behaviors during the setting (such as applying security if need be).
I dont mind either and I don't have strong pref [and dont adhere to some strict standards], just use your judgment. But if you go w/ setters anyone who overrides the class must be aware the object may not be fully created. That being said make sure no extra code in the setters can publish this reference.
Setters can be useful for standard range checks and so on, thus no extra code to verify the input is needed. Again using setters is slightly dirtier approach.
Using the field has obvious benefits (like making sure whoever subclass will not be able to maliciously alter the behavior) and it's usually preferred, esp. in libraries.
Actually c-tor is one the 3 possible way to create an object (serialization + clone are the other 2), some transient state might need to be handled outside the c-tor, so you still have what to ponder about fields vs setters. (Edit, there is another half way: Unsafe but that's what serialization uses)
It's a good question.
As Described here, it's not a Bad Practice, I think there are consideration for both sides.
Use setter vs use this in constructor.
I don't know why this section doesn't described and discussed better in the docs and college courses of OOP, because I found that there is a very big weight on "Getters & Setters", but the constructor in the studying Materials mostly defined by this keyword, and there is a lot of using in protected fields for subclasses.
Is using setters method inside a constructor to initialize a variable a bad practice in Java?
Assuming that by ‘variable’ you mean a field on the instance or class, and that by ‘setter’ you mean a public method which other objects can use to set the value:
No, it’s not bad practice.
If there are business rules that dictate the value of the field must be limited to certain values or validated, then that validation code belongs in the setter. In fact, I would say it’s bad practice for the constructor to bypass the validation code.
However, there are two things you should keep in mind:
As Peter Koves points out, methods called from the constructor must not assume that the object is fully constructed when they are called. But in the case of setters, this isn’t usually a problem, because setters are usually only concerned with setting one field. If a method called by the constructor is overridable, this could result in its work - important to the construction of the object - not being done in a subclass which overrides it. Therefore, it’s good practice to make methods called by the constructor private or final (when the class itself isn’t final). Summary: setters that contain logic that the constructor depends on should be final; any other method that the constructor depends on should be private.
