Does the FindBugs EI_EXPOSE_REP bug only concern Date?

moudug picture moudug · Nov 20, 2012 · Viewed 8.9k times · Source

Findbugs reports a lot of EI_EXPOSE_REP and EI_EXPOSE_REP2 bugs on my code, each time I write getters and setters like this:

  public Date getDate() {
    return date;
  }
  public void setDate(final Date date) {
    this.date = date;
  }

I understand the meaning of the report, I should not expose the internal references of my objet to the outside world so that they can not be modified by a malicious/erronous code. A fix would be:

  public Date getDate() {
    return date == null ? null : date.clone();
  }
  public void setDate(Date date) {
    this.date = date == null ? null : date.clone();
  }

My question is not here. I am surprised that this report concerns ALWAYS Date. Why not all other mutable objects? I think this report goes for all mutable objects too, doesn't it ?

Should I extend this "good practice" to all my accessors that deal with mutable objects?

Give me your advice, thanks

Answer

Brian Agnew picture Brian Agnew · Nov 20, 2012

I would certainly expect this report to relate to all mutable objects, but I suspect that FindBugs is aware of certain common offenders.

I'm normally careful with exposing internal state via getters e.g.

public ArrayList<Trade> getTrades() {
   return trades;
}

means

  1. a client may be exposed to changes in your trade list
  2. a client may change that list that you passed out in good faith

As such there are two approaches.

  1. pass an immutable variant of that object (i.e. an object that can't be changed). In the above scenario you would take a read-only copy of that list and pass that out (you might argue that you could just take a simple read-write copy and pass that since it wouldn't affect the originating object, but that's counterintuitive)
  2. don't pass the object (list of trades) out, but rather have the owning object perform operations on that collection for you. This is perhaps the essence of OO - telling objects to do things for you rather than asking them for info and doing it yourself.

Similar arguments relate to setters and constructor arguments.

Note that you can find yourself copying lots of objects upon exposure in order to protect yourself, and potentially doing a lot of extra work. It's a technique to be used judiciously and it's worth understanding who your client objects are, and if you can control and/or trust them.