Reputation: 16116
I have a list of objects like the next class:
class A {
private String property1;
private String property2;
//Setters && Getters
}
So, after some operations I need to update the list with a default value with some logic like next:
listOfA.forEach(item -> {
item.setProperty1(findSomething());
}
This logic is repeated several times so I'm looking to export it to a method. So, my question is related to this method: Should I update the copy reference of the list with a void method, return it or create new list and update it?:
Option 1: Update the copy reference of the list
private void updateList(List<A> listOfA) {
listOfA.forEach(item -> {
item.setProperty1(findSomething());
}
}
Option 2: Return it
private List<A> updateList(List<A> listOfA) {
listOfA.forEach(item -> {
item.setProperty1(findSomething());
}
return listOfA;
}
Option 3: Create a new list from the other, update this and return it
private List<A> updateList(List<A> listOfA) {
List<A> newList = new ArrayList<>();
//Logic to copy listOfA in newList....
newList.forEach(item -> {
item.setProperty1(findSomething());
}
return newList ;
}
Upvotes: 5
Views: 6868
Reputation: 7526
In the end it is very much personal opinion which option you prefer. There are however some considerations that might help you in the decision process:
The first and second option are virtually the same. Both operate on the List
that is passed in as a parameter. The updating of the list does not create anything new. This would suggest option 1 as the solution to chose, as with only the signature alone (no additional documentation) option 2 might indicate that the returned list is a new List
instance.
Returning the List
as in option 2 and 3, has the advantage that you can execute further operations on that list, which make it changeable.
The last option employs defensive copy to actually create a new instance of the input List
and operate on that list. While it is good practice to do so, it can have some side effects, that may be unwanted. With option 2 it is not required that the returned list is assigned to anything, as it is the same instance that was passed in as parameter. This is not the case of option 3. Here the result must be assigned, otherwise it becomes eligible for garbage collection.
Upvotes: 3
Reputation: 2276
Everything depends on the situation.
Option 1 This case is suitable when you do not need to use your list anywhere else. I mean that if other object has this list reference as member then this object will have this member modified too.
Option 2 This case seems like case 1 and you can use this method as case 1 but here you return your list. It brings some advantage in first case, you can use your method in chain of Stream API or Optionals:
private List<A> updateList(List<A> listOfA) {
List<A> newList = new ArrayList<>();
//Logic to copy listOfA in newList....
newList.forEach(item -> item.setProperty1(findSomething()));
return newList ;
}
public List<A> myMethod() {
List<A> myList = null;
// possible initialization of list
// ...
// here we update list. In the case when list is null then we do not modify it
return Optional.ofNullable(myList).map(this::updateList).orElse(null);
}
Option 3 In this case you copy array into another one. It make sense in the case when initial array should not be modified, for example it is field of some other class.
Upvotes: 1
Reputation: 312
Method :3
when u r making a new list object it needs a certain memory in heap. And then you are just copy the content from your agrument list and update it and return the new list. Here creating a new list object takes an extra headache. No need for it.
Method :2
In this method no new list is created bt you return the same list. While java works on call by value (here values means values of the ref object). You can get the updated list on the calling method not need to return it. While u r returing an object u r increasing metadata of that method.
Method :1
It is the best approach. Here u are using the benefit of call by value (here values means values of the ref object). No unnecessary extra memory is occupied. No unnecessary return. Thats the right approch.
Upvotes: 0