May expose internal representation by incorporating reference to mutable object

Question

I have the following in my code: But why is this giving the Sonar error? Error is on line: this.lastAccessTime = lastAccessTime; The date here is already declared private.

public class myClass{

    @Temporal(TemporalType.TIMESTAMP)
    @Column(name = "LAST_ACCESS_TIME", nullable = false)
    private Date lastAccessTime;

    /**
     * Constructor
     * 
     * @param userId the user id
     * @param screenName the name of screen
     * @param lastAccessTime time of last access
     */
    public userTO(String userId, String screenName, Date lastAccessTime)
    {
        this.userId = userId;
        this.screenName = screenName;
        this.lastAccessTime = lastAccessTime;


    }
}

Powerlord · Accepted Answer · 2014-09-11T14:59:13.080

6

Since Date is a mutable type, the code that passed you the Date can continue to modify it after passing it to your function/constructor.

So, instead of just assigning the Date passed to you, you should instead make a copy of it to prevent this from happening:

       this.lastAccessTime = new Date(lastAccessTime.getTime());

This is covered in Effective Java: Second Edition by Joshua Bloch as Item 39: Make defensive copies when needed.

Note that you should make this copy before doing any validation on the Date as well.
Edit: As noted below, a null check should happen before the copy to prevent a NullPointerException, but other validation should be done after making the copy.

edited Sep 11 '14 at 14:59

answered Sep 11 '14 at 14:50

Powerlord

87,612
17
125
175

Thanks for the tip. When you said "make this copy when doing any validation" do you mean my getter and setter for lastAccessTime? For example my setter would be : this.lastAccessTime = lastAccessTime == null ? null : new Date(lastAccessTime.getTime()); – PhoonOne Sep 11 '14 at 14:53
Well, a `null` check would still have to be done first (or you'll get a `NullPointerException`), but I was thinking more of if you need to validate that the Date is before or after some other Date. This is actually covered in the book I referenced as well. – Powerlord Sep 11 '14 at 14:54
Ok, so if im doing validations later in the code on this Date, i should make a copy of it as well – PhoonOne Sep 11 '14 at 14:55

score 0 · Answer 2 · edited Jun 09 '19 at 05:58

Since java.util.Date is a mutable type, the code that passed you the java.util.Date can continue to modify it after passing it to your function/constructor.

Consider re-factoring to an immutable type i.e. org.joda.time.DateTime

This is covered in Effective Java: Second Edition by Joshua Bloch as Item 15: Minimize mutability.

Note: It looks like you're doing some ORM stuff. jadira Can perform the mapping to joda classes for hibernate.

May expose internal representation by incorporating reference to mutable object

2 Answers2