add a defensive copy of an object to a hashset

Question

Currently I have this code:

public final class Tutor {
private String name;
private final Set<Student> tutees;
public Tutor(String name, Student[] students){
    this.name = name;
     tutees = new HashSet<Student>();
     for (int i = 0; i<students.length; i++)
         tutees.add(students[i]);
}

I'm trying to rewrite it (just on paper) so that it makes/adds defensive copies of students rather than directly adding them into the hashset and am wondering if the following code would do so:

public final class Tutor {
private String name;
private final Set<Student> tutees;
public Tutor(String name, Student[] students){
    this.name = name;
     tutees = new HashSet<Student>();
     for (int i = 0; i<students.length; i++)
         tutees.add(students[i](students.getName(), students.getCourse());
}

Code for Student if needed:

public class Student {
private String name;
private String course;
public Student(String name, String course){
     this.name = name;
     this.course = course;
}
public String getName() { return name; }
public String getCourse() { return course; }
public void setName(String name) {
     this.name = name;
}
public void setCourse(String course){
     this.course = course;
 }
}

thanks

Use a compiler first. It will find at least one error. Next, consider writing a constructor for Student that takes a Student as an argument -- encapsulate what it takes to copy a student in the Student class. — Andy Thomas, May 24 '17 at 14:56

matoni · Accepted Answer · 2017-05-24T15:16:09.557

You are doing it right, but with some mistakes, since you are writing it on paper. If you rewrite it into the program, it would not compile, due of this line

tutees.add(students[i](students.getName(), students.getCourse());

which need to be replaced by

tutees.add(new Student(students[i].getName(), students[i].getCourse());

Note, you are adding new Student, but fields are initilaized by existing references, which results in shallow-copy - objects are different but are sharing the content. However, String class is immutable which means that each method which modifies string creates new string with applied modifications and the old one remains the same. So even if original student and it's copy shares the content, string modifications can not affect each other, therefore we can say it is something that acts like defensive-copy.

Student original = new Student("name", "course");
Student copy = new Student(original.getName(), original.getCourse());
// does not change the name of the copy
String modifiedName = copy.getName().replaceAll("a", "b");

Here is an example of true defensive-copy (deep-copy):

Student deepCopy = new Student(
        new String(original.getName()), 
        new String(original.getCourse())
);

For efficiency reasons, if you know you are working with classes that are immutable, just copy their references.

thanks matoni, i think this sufficiently solves the problem! suspected i'd need a 'new' in there somewhere but couldn't get my head round it :) — pxdr0, May 24 '17 at 15:53

score 1 · Answer 2 · answered May 24 '17 at 15:26

You've identified the problem that putting a mutable student into a Set is a bad idea. You don't want to change something once it's in a set because it breaks the set's contract.

Creating a copy treats the symptoms but it doesn't treat the underlying issue. The issue is that your Student class is mutable. If you make your Student class immutable you don't need to worry about copying and it will be significantly less error prone.

public class Student {
    private String name;
    private String course;
    public Student(String name, String course){
        this.name = name;
        this.course = course;
    }
    public String getName() { return name; }
    public String getCourse() { return course; }
}

If a student changes name - how often does this happen? in your system you may never need to model it at all - or changes course, you just create a new student and remove the old, incorrect one.

thanks michael! i know that student being mutable is an underlying issue, it's just that this particular exercise required student not to be changed. it is, of course, much easier to just address this issue in a real life scenario — pxdr0, May 24 '17 at 15:50

add a defensive copy of an object to a hashset

2 Answers2