CODEWITHSUNDEEP
javareferencepass-by-reference
Jevgeni Smirnov
Jevgeni Smirnov

Reputation: 3797

Why local non-final variable affects the referenced field? Java

I have following code in my program:

...
private void generateStack() {
    List<AdvertisementsModel> adsForModel = Storage.getAdsForId(model.getId());
    ...
    adsForModel.clear();
...

Storage is static class with static fields and methods. generateStack method is in another class and in instance object. Why does adsForModel.clear(); affects the list in Storage class if asdForModel reference is not final?

Upvotes: 0

Views: 96

Answers (4)

Fabian Barney
Fabian Barney

Reputation: 14559

Storage.getAdsForId(...) returns a copy of the reference to the same List object. So calls via this reference effect the same list. When you call Storage.getAdsForId there is no new List created - just a new reference to the same list.

Therefore it's good practise to either return explicitly ImmutableList or making a defensive copy of the list in Storage.getAdsForId(...) and returning the copy.

Be aware that you need to make a deep copy when AdvertisementsModel is mutable or you'll run into the same problem on a different level. (Otherwise you may have a list copy now but both lists still containing references to the same AdvertisementsModel objects and changing them still effect the list contents inside Storage.)

Upvotes: 3

ppeterka
ppeterka

Reputation: 20726

If it is final, is the original list not affected? I have doubts about that... It's the same list instance. For this, I'd use either 

new ArrayList<AdvertisementsModel>(Storage.getAdsForId(model.getId()));

which creates a new list instance and if possible, modified the Storage class to return an UnmofifiableList of the original list:

return Collections.unmodifiableList(adsForIdList);

I'd prefer this, as this solution does not create a new List instance with each call, it is the responsibility of the receiving code to decide if that needs to be created or not. However, in multithreaded environments, if the original list might be modified, this might result in ConcurrentModificationExceptions - so in that case it is wiser to create a "defensive copy" of the list in the getter itself. Be sure to keep in mind, that then the modifications to the copy will not affect the original list...

Upvotes: 1

Chris Knight
Chris Knight

Reputation: 25074

Java is pass by value (of the reference). So, if Storage.getAdsForId(model.getId()) returns a reference which is staticly stored within Storage then calling clear() on it in the instance will affect the same List within Storage as well. You could do this instead:

return new ArrayList<AdvertisementsModel>(Storage.getAdsForId(model.getId()));

to return a copy of the list instead which would avoid affecting the list within Storage. Of course, modifying an element of this list will still affect the same element present within the list in Storage. To avoid that, you'd have to deep copy each element in the list.

Upvotes: 3

Christophe Roussy
Christophe Roussy

Reputation: 17049

getAdsForId should return a copy of the list, otherwise it will return a reference and calling clear on the list will empty the original one.

Upvotes: 1

Related Questions