Should getters return copies of object instances to avoid side effects?

Question

I want to get a value returned from a function of a class.

Inside my class:

public class MyClass {
   private Color color_ = new Color(0f, 0f, 0f, 1f);

   public Color getColor(){
      return this.color_;
   }
}

So, in another class of mine, I call

this.someOtherColorInOtherClass_=myClass.getColor();

If inside myClass the color_ changes though, the color of someOtherColorInOtherClass_ will also change.

I think this is because in Java everything is a pointer. So basically my getter function returns a pointer to the Color and thus they are both pointing to the same memory location, thus changing the same variable.

In my getter function I could do

return new Color(this.color_);

and everything would work fine, as I now have a new pointer to work with.

Is there a better solution to this problem?

Answer 1

Obviously, you could have getters return clones and what not but a more robust solution is...

Immutability

If inside myClass the color_ changes though, the color of someOtherColorInOtherClass_ will also change.

And this both the intended and the expected behavior. But in your case you'd most likely call this a nasty side effect.

This is because Color is a mutable class, ie. you're free to do:

Color color = new Color(r, g, b, alpha1);
...
color.setRGB(r2, g2, b2, alpha2);
...
color.setRGB(r3, g3, b3, alpha3);

thus making your Color instance vulnerable to state changes.

A counter example is the notable String class which is immutable. Immutability is a nice feature that is enforced by a number of (functional) languages like Haskell and Scala and which simplifies concurrent programming and has the added bonus of making state-related matters a no-brainer because of its intrinsic lack of (nasty) side effects.

Indeed, had you modeled the color field of your two classes as String, ie. an immutable class, the issue of sharing the same instance would have been technically safe (and in your case, robust). So the bottom line is, unless you have a good reason to do otherwise, make your classes immutable, especially those intended as parameters. Specifically, that means having rich constructors, no setters or private ones if needed and final fields wherever possible. If your getters return potentially mutable instances (eg. like sets/lists/maps or other objects), return [immutable] copies instead.

For instance, a simplified but immutable Color class could read (with no getters to avoid boilerplate):

public class Color {
    public final double r, g, b, alpha;

    public Color(double r, double g, double b, double alpha) {
        this.r = r;
        this.g = g;
        this.b = b;
        this.alpha = alpha;
    }
}

With this Color class, any number of objects can share the same color instance without potential side effects. HTH