Richard Wimbridge Richard Wimbridge - 3 months ago 15
Java Question

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

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

Answer

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)).