Immutable Java Class with List as field

Question

Can we make the class immutable which has collection as one of the fields?

public class Student implements Comparable<Student> {

    private int rollNumber;
    private String name;
    private Set<String> subjects;
    private List<Integer> marks ;

    public Student(int rollNumber, String name, Set<String> subjects,
            List<Integer> marks) {
        this.rollNumber = rollNumber;
        this.name = name;
        this.subjects = Collections.unmodifiableSet(subjects);
        this.marks = Collections.unmodifiableList(marks);
        setPercentage();
    }

    private float percentage;

    public int getRollNumber() {
        return rollNumber;
    }

    public String getName() {
        return name;
    }

    public Set<String> getSubjects() {
        return new HashSet<>(subjects);
    }

    public List<Integer> getMarks() {
        return new ArrayList<>(marks);
    }

    public float getPercentage() {
        return percentage;
    }

    private void setPercentage() {
        float sum = 0;

        for (Integer i : marks)
            sum = sum + i;
        if (!marks.isEmpty())
            percentage = sum / marks.size();

    }
}

I am not able to achieve it.

I tried:

Set<String> subjects= new HashSet<>();
subjects.add("Maths");
subjects.add("Science");
subjects.add("English");
List<Integer> marks1= new LinkedList<Integer>();
marks1.add(45);
marks1.add(36);
marks1.add(98);
Student student1= new Student(1, "Payal", subjects, marks1);
//student1.getSubjects().add("History");
subjects.add("History");
System.out.println(student1);

But subjects.add is changing the state of the object.

Please help.

Answer 1

You're making a copy of both collections before returning them from your getters. This is unnecessary, since the collections are unmodifiable (unless you want the caller to get mutable collections and not unmodifiable ones).

What is necessary is to make copies of the collections that are passed from the outside in the contructor. Otherwise, the caller can still modify the collections after they've been stored in your object:

this.subjects = Collections.unmodifiableSet(new HashSet<>(subjects));
this.marks = Collections.unmodifiableList(new ArrayList<>(marks));

To be truly immutable, the class and its fields should also be final.