Do I need defensive copying constructing an immutable class with *non final* albeit immutable fields?

Question

One should not expose the reference to a non final field if one wants to construct an immutable class - but even for immutable objects like Strings ?

public final class Test { // Test class is meant to be immutable

    private String s; // CAN'T MAKE THIS FINAL

    void onCreate(String s) { // a callback called ONCE after construction
        this.s = new String(s); // do I need to do this ? (protect me from me)
    }

    public String getS() {
        return new String(s); //do I need to do this ?(protect me from the world)
    }
}

Answer 1

It does not matter whether this class is immutable (for any definition of immutable). In particular, it does not matter if the reference s is ever changed to point to a different string. The string object is immutable, so you don't need to copy it. Without defensive copying, callers of getS will get references to the same string object used by Test's methods and by other callers of getS. That does not matter because nothing¹ they do to this string will affect other referents. It'd be a waste of time and memory.

¹ I'm ignoring reflection. Code that maliciously uses reflection like this can break almost anything, and is not written by accident or hard to spot. It is not even remotely practical to worry about this case.

Answer 2

In theory it is possible through unsafe publication to see an instance of the Test class with an uninitialised (null) s which can also be seen with a correctly initialised s. This is fixable by making s volatile.

However, if you've got some callback happening like that, I think you want to take another look at your design.

If you were to make the class Serializable then you'd have many more problems.

Answer 3

I don't think it's necessary. Even in the documentation is said:

Strings are constant; their values cannot be changed after they are created. Because String objects are immutable they can be shared.

So once a String object is created, its value is never changed. If we want to "change" the value of variable a new String object is created. Such as in the toUpperCase method the original string is unchanged, but a new copy is created.

EDIT:

And when considering strings, literals are put into a shared pool, which means that:

String h = "HELLO";
String h1 = "HELLO";

both s1 and s2 refer to the same object.

you can try that following code returns true:

String h = "HELLO";
String h1 = "HELLO";
boolean r = (h==h1);
System.out.println(r);

However you could change the value of the String's value using reflection:

java.lang.reflect.Field valueField = String.class.getDeclaredField("value");
valueField.setAccessible(true);
valueField.set("Original", "Modified".toCharArray()); 

Answer 4

Technically, if you really want a immutable class in Java you have to make sure that an instance of your class can not be changed after it's created. Therefore all its fields can be final and if they are "exposed" to the world via getters, for example, those fields must either be immutable themselves (as strings are) or not being returned to the outer world (kept private and creating defensive copies of them in getters), so the original field value stays the same. This immutability must not be prone to being broken by inheriting from this class as well.

You can read more about it in Effective Java - a book by Joshua Bloch, or take some notes from the internet, like from here.

Regarding your recent update to the post, here's a suggestion that ensures the initialization was only made once:

private String s; // CAN'T MAKE THIS FINAL
private boolean stringWasSet = false;

public void onCreate(String s) { // a callback called ONCE after construction
    if (!stringWasSet) {
        this.s = s; // No need for defensive copy here, if the variable itself is immutable, like String
        stringWasSet = true;
    }
}

public String getS() {
    return s; // No need for defensive copy here, if the variable itself is immutable, like String
}