When I add to arrayList, it is adding the main notes list not the objects

Question

I have a Person object that has been initialized and if I call the method like getName I can retrieve them.

public Person( String fName, String lName, List<Role> roleList) {
    this.firstName = fName;
    this.lastName = lName;
    this.roleList = new ArrayList<Role>();
}

Each Person needs to have a list of roles so I have a list called roleList which I initialize using List<Role> roleList = new ArrayList();

when created it is passed as

testDriver = new Person("Mike", "Joy", testRoleList);
testDriver2 = new Person("Rich", "Johns", testRoleList);

In my calling class i want to add an item only to that staff members testRoleList. For this test i have made testRoleList public but I do have a getter which retrieves the testRoleList;

Role roleToAdd = Role.DRIVER;

// Only add the new role to the test driver
testDriver.testRoleList.add(roleToAdd);

But when you run this it does add the role, but if I try to retrieve the size of the testDriver and testDriver2 list using:

System.out.println(testDriver.testRoleList.size());

System.out.println(testDriver2.testRoleList.size());

The resulting list size is 1 for both list even though I haven't added a role to the testDriver2.

How can I just add the role to just testDriver or Just testDriver2

As we haven't the complex code (testDriver is a Person object that didn't have a testRoleList but a roleList property) I'm pretty sure it's just a bug in the code ... — loicmathieu, Aug 16 '16 at 12:45
It's actually not a problem of call-by-value vs. call-by-reference. The problem seems to be somewhere in the `Person` class, e.g. `roleList` being static or sth. like that. Also you're initializing `roleList` in the constructor but add the element to `testRoleList` which might just be a totally different (and shared) list. — Thomas, Aug 16 '16 at 12:46
I have tried it now by saying in the Person constructor this.testRoleList = roleList which is the parameter it is passed as its created. but i still get the same list size it seems to be adding only to testRoleList and not the property of it inside the specific person. — Richard Wimbridge, Aug 16 '16 at 13:03

Zabuzard · Accepted Answer · 2016-08-16T13:40:28.240

The thing is that whenever you create a new Person, the passed List will be overwritten with a freshly created new empty List. So if you add something to the list, for example by:

testDriver.testRoleList.add(roleToAdd);

And after that create a new Person:

testDriver2 = new Person("Rich", "Johns", testRoleList);

Then the list will be empty again, because you have this code in the constructor:

this.roleList = new ArrayList<Role>();

You need exactly to know what your code does, change it so it does what you want. At this point it may be good to say that it is not a good design to change passed parameters by overriding them.
If you want something like a global List of roles, then it is not the task of the class Person to initialize this List. Let some other class do this job, pass the reference to the list to Person as you already do and then do not override it with a new reference. Save it, use it, add something, but not recreate it.

But if you want every Person to have its own List of roles, then there is no need to pass a reference from outside. In this scenario it is the task of the class Person and only of this class to manage such a List.
In this case you should remove the List parameter from the constructor but create a List as member variable, then offer some methods to interact with it. Here's an example:

public final class Person {

    private final List<Role> mRoleList;

    public Person(final String fName, final String lName) {
        ...
        mRoleList = new LinkedList<>();
    }

    public void addRole(final Role role) {
        mRoleList.add(role);
    }

    public boolean hasRole(final Role role) {
        return mRoleList.contains(role);
    }

    public List<Role> getRoles() {
        return Collections.unmodifiableList(mRoleList);
    }

}

Then you can do such stuff:

Role r1 = ...
Role r2 = ...

Person person1 = new Person("a", "a");
Person person2 = new Person("b", "b");

person1.addRole(r1);
person1.addRole(r2);

person2.addRole(r2);

And the result is what you would expect, person1 has r1 and r2, person2 only has r2.
As side note, maybe a Set instead of a List reflects better what you want your roleList to be. Queries like contain then are very fast (O(1)), in a List they are very slow (O(n)).

score 0 · Answer 2 · edited Aug 16 '16 at 14:53

In simple words, you are referring same object testRoleList, in both of your objects testDriver, testDriver2.

When constructing the Person object, pass List of Roles by constructing different List holding Role for each Person object.

Set is better than a List from performance point of view.

If Roles are limited and well decided, you could use EnumSet in place of ArrayList. Here also, you need to initialize different EnumSet "Role" objects for different Person objects.

To avoid Synchronization issues, use Collections.synchronizedSet.

Set<MyEnum> s = Collections.synchronizedSet(EnumSet.noneOf(MyEnum.class));

Refer https://docs.oracle.com/javase/8/docs/api/java/util/EnumSet.html

When I add to arrayList, it is adding the main notes list not the objects

2 Answers2