Java: Is setters should make `Clone` or it's up to caller pass `new` copy?

Question

What considered to be the best & common practice if I want to make sure that after setter is called the object cannot be modified from outside? In the code there is detailed simple self explained example, With 2 options dilemma.

//caller scope
CustomObject original = new CustomObject(params...);  //original state 1
MyClass mMyClass = new MyClass(original);
original.modifyMe(params...);  //original state 2
mMyClass.setCustomObject(original);
original.modifyMe(params...); //original state 3


/*!!!REQUIREMENT: mMyClass.CustomObject should be in state 2!!!*/


class MyClass {

    private CustomObject mObject;

    public MyClass() {
        this.mObject = new CustomObject();
    }

    public MyClass(CustomObject obj) {
        this.mObject = obj.Clone();
    }

    //mObject is private, modified only through setter
    public getCustomObject() {
        return this.mObject;
    }

    public setCustomObject(CustomObject obj) {
        //Option 1 in the caller  
        //mMyClass.setCustomObject(new CustomObject(params...));
        this.mObject = obj;

        //Option 2 in the caller
        //mMyClass.setCustomObject(callerCustomObject);
        this.mObject = obj.Clone();
    }

}

If you want to make sure that the object can't be modified outside of the class, you have to make copies *both in and out*. But don't use `clone` - the object should have a copy constructor. — RealSkeptic, Sep 19 '15 at 09:29
@RealSkeptic 1. Why use copy in get? I don't know maybe from outside they want just to use not modify the object, I think it's better to make the check in the in direction. 2. What you mean copy constructor in java? this is not why we need Clone()? — michael, Sep 19 '15 at 09:32
If you `get` the object, then you can change it and it will change the object inside the class. So always copy both in and out. `clone()` is [broken](http://stackoverflow.com/a/2427946/4125191). — RealSkeptic, Sep 19 '15 at 09:34
@RealSkeptic I implement the Clonebale interface and override the clone with copy constructor style - is this the best practice? — michael, Sep 19 '15 at 09:35
The best practice is not to implement `Cloneable` and not to override `clone`. — RealSkeptic, Sep 19 '15 at 09:36
@RealSkeptic In the university course that's what we learned - in java use clone. What is the alternative? — michael, Sep 19 '15 at 09:37
@michael [Real Skeptic](http://stackoverflow.com/questions/32666502/java-is-setters-should-make-clone-or-its-up-to-caller-pass-new-copy#comment53178827_32666502) is absolutely right. You need to make copies both in and out for immutability. You should never use `clone()`, it's a mistake in the Java API. Both of these points are covered in [Effective Java](http://www.amazon.co.uk/Effective-Java-Edition-Joshua-Bloch/dp/0321356683); which is required reading for all Java developers. — Boris the Spider, Sep 19 '15 at 09:39
@BoristheSpider I understood the point with in & out copies. My question is regarding alternative to clone() in order to make a copy. What you are basically suggest is to make my custom copy method in the rule of copy constructor/what clone() intended to be? — michael, Sep 19 '15 at 09:44
@michael Bloch also covers recommended alternatives. A `static` method that creates an instance and copies properties across is often a good choice, but a copy constructor is more common. — Boris the Spider, Sep 19 '15 at 09:49

score 1 · Accepted Answer · answered Sep 19 '15 at 10:37

I wouldn't use clone here. Rather than making inefficient defensive copies, try making CustomObject immutable. You can modify state by adding withXXX methods (roughly equivalent to setXXXX) but they create a new instance of the host object (rather than the Object being passed in). Project lombok comes with some handy preprocessor annotations for creating Immutable objects with Withers. Also see the Immutables 2.0 project.

@AllArgsConstructor
class CustomObject {

   @Wither @Getter
   private final int state;

}

CustomObject one = new CustomObject(1);
CustomObject two = one.withState(2);

assertThat(one.getState(),equalTo(1));
assertThat(two.getState(),equalTo(2));

By using genuine Immutable Objects you will incur much less memory (& GC) and CPU overhead than with defensive copies - as well as much simpler code.

Java: Is setters should make `Clone` or it's up to caller pass `new` copy?

1 Answers1