CODEWITHSUNDEEP
javaimmutability
Jay
Jay

Reputation: 51

How to make the below java class Immutable

Is there anything else to do to make the below class immutable apart from the following.

Upvotes: 2

Views: 213

Answers (6)

Grundlefleck
Grundlefleck

Reputation: 129207

As others have correctly answered, you need to ensure nobody can modify the listOfNumbers field.

However, you could have got the same answer using an automated tool I wrote called Mutability Detector, and it may come in handy when you want to test other classes you want to make immutable.

Given your ExampleClass, and the following unit test:

import org.junit.Test;
import static org.mutabilitydetector.unittesting.MutabilityAssert.assertImmutable;

public class Question_30240358 {

    @Test
    public void isImmutable() {
        assertImmutable(ImmutableClass.class);
    }
}

The result is a failed unit test, with the following message:

Expected: org.mutabilitydetector.stackoverflow.ImmutableClass to be IMMUTABLE
     but: org.mutabilitydetector.stackoverflow.ImmutableClass is actually NOT_IMMUTABLE
    Reasons:
        Attempts to wrap mutable collection type using a non-whitelisted unmodifiable wrapper method. [Field: listOfNumbers, Class: org.mutabilitydetector.stackoverflow.ImmutableClass]
    Allowed reasons:
        None.
        at org.mutabilitydetector.unittesting.internal.AssertionReporter.assertThat(AssertionReporter.java:48)
        at org.mutabilitydetector.unittesting.MutabilityAsserter.assertImmutable(MutabilityAsserter.java:108)
        at org.mutabilitydetector.unittesting.MutabilityAssert.assertImmutable(MutabilityAssert.java:672)
        at org.mutabilitydetector.stackoverflow.Question_30240358.isImmutable(Question_30240358.java:14)

This test will pass if the field assignment is changed to:

this.listOfNumbers = Collections.unmodifiableList(new ArrayList<Integer>(listOfNumbers));

The test will catch many other kinds of issues that introduce mutability.

Upvotes: 1

biziclop
biziclop

Reputation: 49714

Yes. You need to make a defensive copy of the list provided in the constructor and another one in the getter. Alternatively, using Google Guava's ImmutableList class you can save the copying in the getter.

public ImmutableClass(String name, List<Integer> listOfNumbers) {
    this.name = name;
    // you need to null check your parameter here first
    this.listOfNumbers = ImmutableList.copyOf(listOfNumbers);
}

This ensures that the object returned by the getter can't be tampered with by the client, even though it's the same object instance you store in the field.

If you want to be really pedantic, you can still write your getter like this with relatively little overhead:

public List<Integer> getListOfNumbers() {
    return ImmutableList.copyOf(listOfNumbers);
}

As ImmutableList.copyOf() will try to avoid making a copy when it's safe to do so, this won't actually create a new copy so there isn't much point putting it in.

P.s.: It's also good practice to check your input parameters in the constructor against any preconditions you may want to enforce. (For example that the list mustn't be null and mustn't be empty.) These checks should always be done on the copy except for the null check, which needs to happen before creating the copy. But the point of this is not so much immutability but writing secure code that maintains its invariants however the client tries to break them.

Upvotes: 6

TimoStaudinger
TimoStaudinger

Reputation: 42460

Have a look at Joshua Bloch's book Effective Java, specifically Item 15.

He gives an amazing explanation on how to make a class immutable.

Upvotes: 2

dejvuth
dejvuth

Reputation: 7136

Use Collections.unmodifiableList

public List<Integer> getListOfNumbers() {
    return Collections.unmodifiableList(listOfNumbers);
}

Upvotes: 2

Robby Cornelissen
Robby Cornelissen

Reputation: 97120

The listOfNumbers needs to be copied in the constructor, and the getter for the list needs to return a copy of the list. As it stands, you're returning a mutable list which violates the immutability of the class.

Alternatively, you could use an immutable list implementation, e.g. the one from Guava.

Upvotes: 2

nomis
nomis

Reputation: 2715

You should defensive-copy the listOfNumbers passed into the constructor and return an immutable view of it in the getter.

Upvotes: 6

Related Questions