patternjavaMajor
Immutable objects in Java
Viewed 0 times
javaimmutableobjects
Problem
I just finished studying immutable objects and their advantages so I thought I'd create one of my own. Here is my employee class that extends a person class. It is not mutable since I have getters to my mutable date objects.
The designation enum:
The Oracle Java documentation says:
Don't share references to the mutable objects. Never store references
to external, mutable objects passed to the constructor; if necessary,
create copies, and store references to the copies. Similarly, create
copies of your internal mutable objects when necessary to avoid
returning the originals in your methods
package objects.objects;
import java.util.Date;
public class Person {
private final String name;
private final Date dateOfBirth;
Person(String name,Date dateOfBirth){
this.name=name;
this.dateOfBirth=dateOfBirth;
}
public String getName() {
return name;
}
public Date getDateOfBirth() {
return dateOfBirth;
}
}Employee Class:package objects.objects;
import java.util.Date;
public final class Employee extends Person{
private final String empolyeeID;
private final Designation designation;
private final double salary;
private final Date dateOfJoining;
public Employee(String Name,Date dateOfBirth,String employeeID,Designation designation,double salary,Date dateOfJoining)
{
super(Name,dateOfBirth);
this.empolyeeID=employeeID;
this.designation=designation;
this.salary=salary;
this.dateOfJoining=dateOfJoining;
}
public String getEmpolyeeID() {
return empolyeeID;
}
public Designation getDesignation() {
return designation;
}
public double getSalary() {
return salary;
}
public Date getDateOfJoining() {
return dateOfJoining;
}
}The designation enum:
package objects.objects;
public enum Designation {
ASSOCIATE,SENIOR_ASSOCIATE,MANAGER,SENIOR_MANAGER,DIRECTOR
}The Oracle Java documentation says:
Don't share references to the mutable objects. Never store references
to external, mutable objects passed to the constructor; if necessary,
create copies, and store references to the copies. Similarly, create
copies of your internal mutable objects when necessary to avoid
returning the originals in your methods
Solution
All your fields are private and final, which is the first step to being immutable. This is good.
The Date class is not immutable, so, for example, you have a problem with the method:
because, someone could do:
and suddenly your employee has aged... a lot.
What the oracle documentation is saying, is that, if you have mutable content, you should return a copy of that data. This is often called a 'defensive copy':
Update: additionally, your method should be
Note, that the constructor for
Apart from that, the immutability looks fine. Good, even.
The package name you have chosen though... is lacking:
The Date class is not immutable, so, for example, you have a problem with the method:
public Date getDateOfBirth() {
return dateOfBirth;
}because, someone could do:
employee.getDateOfBirth().setYear(1900);and suddenly your employee has aged... a lot.
What the oracle documentation is saying, is that, if you have mutable content, you should return a copy of that data. This is often called a 'defensive copy':
public Date getDateOfBirth() {
return new Date(dateOfBirth.getTime());
}Update: additionally, your method should be
final so that no subclasses can change the behaviour of getDateOfBirth() (same is true for getName()). If the method is not final, then a subclass could possibly override the method, and do it in a way which make the name/dateOfBirth mutable.Note, that the constructor for
Date, in this instance, takes a long value. This leads on to a suggestion that the immutable class should store a long instead of a Date. The long is immutable anyway, if final.public class Person {
private final String name;
private final long dateOfBirth;
.....
public Date getDateOfBirth() {
return new Date(dateOfBirth);
}
}Apart from that, the immutability looks fine. Good, even.
The package name you have chosen though... is lacking:
objects.objects. You could not name your package something better?Code Snippets
public Date getDateOfBirth() {
return dateOfBirth;
}employee.getDateOfBirth().setYear(1900);public Date getDateOfBirth() {
return new Date(dateOfBirth.getTime());
}public class Person {
private final String name;
private final long dateOfBirth;
.....
public Date getDateOfBirth() {
return new Date(dateOfBirth);
}
}Context
StackExchange Code Review Q#66629, answer score: 33
Revisions (0)
No revisions yet.