Class best practice

Question

If I have a customer class with an overloaded constructor (default and one with params) what is the proper way to set the class members in the Overloaded constructor? Using "this" references or using the setter methods?

Just wasn't sure what the proper method was.

public class Customer {

private String firstName;
private String lastName;
private int age;

public Customer() {}

//This Way
public Customer(String firstName, String lastName, int age)
{
    this.firstName = firstName;
    this.lastName = lastName;
    this.age = age;
}

// Or this way?
  public Customer(String firstName, String lastName, int age)
{
    setFirstName(firstName); 
    setLastName(lastName);
    setAge(age);
}



/**
 * @return the firstName
 */
public String getFirstName() {
    return firstName;
}

/**
 * @param firstName the firstName to set
 */
public void setFirstName(String firstName) {
    this.firstName = firstName;
}

/**
 * @return the lastName
 */
public String getLastName() {
    return lastName;
}

/**
 * @param lastName the lastName to set
 */
public void setLastName(String lastName) {
    this.lastName = lastName;
}

/**
 * @return the age
 */
public int getAge() {
    return age;
}

/**
 * @param age the age to set
 */
public void setAge(int age) {
    this.age = age;
}

}

score 27 · Accepted Answer · answered Sep 07 '12 at 20:16

27

The first one (using this.) is probably safer and more straightforward. Consider if a future subclass overrode the setter methods - this could cause very unexpected behavior.

If your class is final, this is irrelevant and it's a wash.

answered Sep 07 '12 at 20:16

Joe K

18,204
2
36
58

4

This is the only sensible answer so far. Several lint tools will, in fact, warn about calling non-final methods from constructors because of exactly this problem. (If the class is not final but the setter methods are, it's just as good.) – Ted Hopp Sep 07 '12 at 20:17
@KumarVivekMitra - I don't like your answer, as my comment to it indicates. – Ted Hopp Sep 07 '12 at 20:31
@TedHopp please read my answer again...i think we both are talking abt the same thing..... still let me again write it over here...I think you are taking about `OCP` principle,,, Classes are open for extension, but closed for modification.... And as I know `Constructors canNot be inherited, and so CANNOT be overridden, so its Immutable....` – Kumar Vivek Mitra Sep 07 '12 at 20:36
@KumarVivekMitra - If a class has a setter, then it is mutable (or else what does the setter do). The terms "mutable" and "immutable" don't apply to constructors, as far as I know. – Ted Hopp Sep 07 '12 at 20:40
@TedHopp I am `not talking abt the constructor` to be immutable, `i am talking abt the Clas`s..... its very clearly written in my answer... – Kumar Vivek Mitra Sep 07 '12 at 20:41
+1. Much more on this, including authoritative references, at http://stackoverflow.com/a/3404369/978917. – ruakh Sep 07 '12 at 20:56
Thanks Joe I appreciate the info – scarpacci Sep 07 '12 at 21:02

Kumar Vivek Mitra · Answer 2 · 2012-09-07T20:31:09.447

2

Its Not about which is a better way to do it, but what you want out of it......

- If you want your class to be mutable, then go with setters.

- If you want your class to be Immutable then i think using this, is a better option.

I think using this, is apt in places, where you receive some data from a webserver or some source, then store them into an Collection in form of Instances of a Custom class.....

Eg:

Create a Class Student,
As you make a request to some web-service, you will get the response, for eg: JSON...
Parse it, then Create an Instance of Student and Store it in an Collection..

eg:

ArrayList<Student> arList = new ArrayList<Student>();

arList.add(new Student(name,rollNos,class,marks));

edited Sep 07 '12 at 20:31

answered Sep 07 '12 at 20:25

Kumar Vivek Mitra

33,294
6
48
75

Calling a non-final method (such as a setter) from a constructor is not a good idea. See, e.g., [this article](http://www.informit.com/articles/article.aspx?p=20521). – Ted Hopp Sep 07 '12 at 20:30
@TedHopp I think you are taking about `OCP` principle,,, Classes are open for extension, but closed for modification.... And as I know `Constructors canNot be inherited, and so CANNOT be overridden, so its Immutable....` – Kumar Vivek Mitra Sep 07 '12 at 20:35
See [this](https://www.securecoding.cert.org/confluence/display/java/MET05-J.+Ensure+that+constructors+do+not+call+overridable+methods) for a good explanation of why one should never call an overridable method from a constructor. Most lint tools will complain about calling setter methods from a constructor (unless the method or class is final, or the method is private--in which case the method cannot be overridden). – Ted Hopp Sep 07 '12 at 20:43
@TedHopp, i think there is some confusion between us...i am `not taking abt calling overridden methods from a constructor`, but calling `other Constructors with different arguments from within the constructors` using `this`.. – Kumar Vivek Mitra Sep 07 '12 at 20:46
Ah. That I have no objection to. However, it has nothing to do with OP's question. – Ted Hopp Sep 07 '12 at 20:46
I just gave this knowledge as an addon to the OP – Kumar Vivek Mitra Sep 07 '12 at 20:47

score 1 · Answer 3 · answered Sep 07 '12 at 20:18

The best answer would be "depends". Generally, you don't need to use setters unless setters do something more like a calculation before setting the value. However, if your setters only set the value directly, then this is probably best for you. On the contrary, setters are used for validation and stuff, if you use this, you'll miss out on them.

score 0 · Answer 4 · answered Sep 07 '12 at 20:16

Basically, there is no difference except for validation.

On one hand, if your setter validates new values based only in the new values (they do not depend of the object state), calling the setter avoids duplicating the validation logic.

On the other hand, if the validation of one setter checks other properties, then you either must be sure that the needed properties are already set before calling that setter, or assign the property directly.

score 0 · Answer 5 · answered Sep 07 '12 at 20:17

0

The direct access way is faster (7 times faster if memory serves me correctly) but the second one is "safer" in case some special rules to assigning those values or chaining of assignment (where you assign a second field from the setter of another) where implemented in the accessors, that is unless of course you want to explicity bypass those rules/chainings when the assignment is done from the constructor.

So in short, it depends on your needs but those are the main concerns I can think of.

answered Sep 07 '12 at 20:17

JTMon

3,189
22
24

The second method is far from "safer" unless the setter methods (or the class) are `final`. What if a subclass overrides the setter and fails to call up to the superclass? – Ted Hopp Sep 07 '12 at 20:18
1

If you're subclassing a type and are overriding setter methods without setting anything with them then you've got a much bigger problem than deciding which constructor to use. – Sep 07 '12 at 20:22
@TedHopp I was writing a long description of what Jordan White said so I will leave it at that :) – JTMon Sep 07 '12 at 20:23
@JordanWhite - I think you misread the question. There's no "base class" issue involved here. The variables being set are private to the class. OP used "base" where "default" was more appropriate. – Ted Hopp Sep 07 '12 at 20:26
@TedHopp I read the question just fine and cannot envision any sane situation where someone would inherit a class only to override the setter methods and not update the superclass. – Sep 07 '12 at 20:32
@JordanWhite - The reason most lint tools will complain about calling non-final methods from constructors is because it's just not a safe practice. Even sane programmers sometimes make mistakes; a missing call to `super.setWhatever()` in some random subclass can be very difficult to track down (especially if the symptoms only appear much later during execution). – Ted Hopp Sep 07 '12 at 20:39
@TedHopp Ah, I wasn't advocating for either approach I was simply touching on the bit about overriding setters. Personally I would set the variables directly unless extensive validation was required. As for missing calls to the super, easily verified by a short test case. – Sep 07 '12 at 20:43
@JordanWhite - Granted that testing should find a missing super call. However, another (and potentially more serious) problem with calling an overridden setter is that the subclass constructor will not have run yet when the setter is called from the superclass constructor. I can imagine lots of (otherwise sane) situations where the subclass's setter depends on the subclass constructor having already executed. – Ted Hopp Sep 07 '12 at 21:08
@TedHopp Yes overriding a setter will call it before the rest of the constructor in the subclass but these are things that ultimately come down to your desired use cases. Originally I was merely speaking of overriding the super's setters without fulfilling their original intent unless you plan to supersede that functionality. – Sep 07 '12 at 21:21

score 0 · Answer 6 · answered Sep 07 '12 at 21:16

As was already stated it's quite hazardous to call overridable methods in constructor. However if you need to perform some sort of validation that takes place in the setter you still have a few sensible ways to achieve it.

Make the setter final. The fastest solution. This one has been already stated but it isn't the only option.
Use private copy of the setter.
Use factory method instead of constructor. I usually stick with this one. Because this way you have more freedom to handle the situation if the validation fails and it communicates intent better than (2)

Class best practice

6 Answers6