3

I have a java class:

class MyObj{
  private Timestamp myDate;

  public  Timestamp getMyDate(){ 
       return mydate;
  }
  ...
}

when I check it by Findbugs, it says:

Bug kind and pattern: EI - EI_EXPOSE_REP May expose internal representation by returning reference to mutable object

so, what is the better way to write the getter for Date and Timestamp types in Java?

worker_bee
  • 451
  • 4
  • 11
Yu Jiaao
  • 4,444
  • 5
  • 44
  • 57
  • 2
    Even better than the answers below would be to use the new date and time classes in the package `java.time`, which are immutable, instead of the old `java.util.Date`, `java.sql.Date` and `java.sql.Timestamp` classes. – Jesper Jul 12 '17 at 07:13
  • Fully upgrade to Java 8 time classes requests times ;) – Yu Jiaao Jul 12 '17 at 07:44
  • Please read [Why is asking a question on “best practice” a bad thing?](https://meta.stackexchange.com/questions/142353/why-is-asking-a-question-on-best-practice-a-bad-thing/243450) before attempting to ask more questions that are opinion based that invite argumentative discussion because they do not have a single agreed upon answer. –  Jul 12 '17 at 15:24
  • Maybe, maybe not ask `best practice` is the *best practice* – Yu Jiaao Jul 13 '17 at 01:42

2 Answers2

8

Date and Timestamp are both mutable, so returning a reference to your Timestamp means the caller can change the internal state of your class. That's only a problem if it's a problem, if that makes sense; if you mean for the caller to be able to modify the state of your object (by modifying the state of the instance field you're returning), that's okay, though it can be the source of relatively subtle bugs. Often, though, you don't mean to allow the caller to do that; hence FindBugs flagging it up.

You have a couple of choices if you want to avoid exposing a reference to a mutable object:

  1. Clone the object when returning it (a "defensive copy"), so that the caller gets a copy, not the original:

    public Timestamp getMyDate(){ 
     return new Timestamp(mydate.getTime());
}

  2. Return an immutable type or a primitive instead of a mutable one, for instance:

    public long getMyDate(){ 
     return mydate.getTime();
}

  3. Don't use a mutable object at all. For instance, instead of Timestamp, you might use LocalDateTime or ZonedDateTime from java.time, e.g.:

    class MyObj{
  private LocalDateTime myDate;

  public LocalDateTime getMyDate(){ 
       return mydate;
  }
  // ...
}

    If you need to update the date in your class (let's use the example of adding a day), instead of mutating the object, you replace it with a new one:

    this.mydate = this.myDate.plusDays(1);
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • @BasilBourque: Thanks for pointing that out, I haven't used the `java.time` stuff in anger. Silly to have the interface if it's not meant to be used. I've updated the answer. – T.J. Crowder Jul 12 '17 at 15:06
6

Return a defensive copy rather than original so that the getter accessor can't modify your actual Timestamp .

 public Timestamp getMyDate(){ 
       return new Timestamp(mydate.getTime());
  }
Suresh Atta
  • 120,458
  • 37
  • 198
  • 307