Search code examples
javacoding-styleclonegetter-settercloneable

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


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();
    }

}

Solution

  • 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.