Update parameters of a list in a method: return list or not

Question

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 ;
 }

Answer 1

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.

Answer 2

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.