16

Why do i get different behaviors with:

  1. Collection col2 = new ArrayList(col);

  2. Collection col2 = new ArrayList();
    col2.addAll(col)

I'm working with viewers, and the code is complex, and i'm trying to explain the "root" of the problem. Another interesting fact is the next one...

//IF i use this code i have the correct behavior in my app:
public void updateCollection(Collection<Object> col) {
    this.objectCollection.clear();
    this.objectCollection.addAll(col);
}

//IF i use this code i have unexpected behavior in my app:
public void updateCollection(Collection<Object> col) {
    this.objectCollection=new ArrayList(col);
}
marcolopes
  • 9,232
  • 14
  • 54
  • 65
  • 8
    please clarify, which exact behavior is confusing for you? – Neeme Praks Nov 21 '10 at 15:36
  • My code is having different behaviors using a) or b). In my mind, both operations will lead to the same result, but apparently they do NOT. Something is different. – marcolopes Nov 21 '10 at 15:55
  • 2
    You're still too vague. What happens? What happens not? Please post an [SSCCE](http://sscce.org) along with the (un)expected results at your environment. – BalusC Nov 21 '10 at 15:56
  • 2
    Something is wrong with your understanding. `new ArrayList(col)` *works*. You have another problem and have not given us enough information to determine what it might be. – Carl Manaster Nov 21 '10 at 16:18
  • i'm following logic... if i do "new ArrayList" the code does NOT work. With "clear & AddAll" it Does. That's the only thing i change in the code. – marcolopes Nov 21 '10 at 16:25
  • what is that "unexpected behavior"? Sounds like some other code keeps a reference to that particular collection instance and keeps on using that collection even if you have assigned a new collection instance to the field. I guess "objectCollection" field is not private, is it? – Neeme Praks Nov 21 '10 at 16:32
  • 3
    Looking at your edit, in the case of the working code, you are adding the information to an already-existing ArrayList. In the case of the non-working code, you're creating a new one. If, for instance, another entity has direct access to the pre-existing ArrayList, the second bit of code won't be changing that entity's copy of it. Maybe that's your problem. – Carl Manaster Nov 21 '10 at 16:35

3 Answers3

17

This code works:

public void updateCollection(Collection<Object> col) {
    this.objectCollection.clear();
    this.objectCollection.addAll(col);
}

But this introduces problems:

public void updateCollection(Collection<Object> col) {
    this.objectCollection = new ArrayList(col);
}

I suspect that this variation on your first method would introduce identical problems:

public void updateCollection(Collection<Object> col) {
    this.objectCollection = new ArrayList();
    this.objectCollection.clear();
    this.objectCollection.addAll(col);
}

Why? Evidently you have another reference to objectCollection in use somewhere. Somewhere in your code, another object is saying (for instance):

myCopyOfObjectCollection = theOtherObject.objectCollection;

If you're using a getter, that doesn't change the underlying behavior - you are still keeping another reference around.

So if on initial assignment, say, the collection contained {1, 2, 3}, you start out with:

  • this.objectCollection: {1, 2, 3}
  • that.copyOfObjectCollection: {1, 2, 3}

When you assign a new ArrayList to this.objectCollection, and populate it with, say, {4, 5, 6}, you get this:

  • this.objectCollection: {4, 5, 6}
  • that.copyOfObjectCollection: {1, 2, 3}

So that is still pointing to the original ArrayList.

informatik01
  • 16,038
  • 10
  • 74
  • 104
Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
7
Collection col2 = new ArrayList(col);

will create a new ArrayList with size col.size() (+10%) and copy all elements from col into that array.

Collection col2 = new ArrayList();

will create a new ArrayList with initial size of 10 (at least in Sun implementation).

col2.addAll(col);

will copy all elements from col into the end of the col2 ArrayList, enlarging the backing array size, if needed.

So, depending on your col collection size, the behavior will be a bit different, but not too much.

It is preferable to use the first option - that will avoid at least one extra backing array expansion operation.

Neeme Praks
  • 8,956
  • 5
  • 47
  • 47
0
    public List getAdminImIdsWithValidShortNames(){
    return adminImIdsWithValidShortNames;
}

public void setAdminImIdsWithValidShortNames(List adminImIdsWithValidShortNames){
    this.adminImIdsWithValidShortNames=adminImIdsWithValidShortNames;
}

I think, easy is beautiful, just generator setter/getter method is a good habit. if you first clear, then addAll, the list need to clear all elements of list, then addAll will be extra backing array expansion operation, that's not science.

just replacement, this variable will be point to new List, the old list will be auto-GC.

zg_spring
  • 349
  • 1
  • 10