Return list or modify by reference

Question

In java, I have a method which is modifying the contents of a list. Is it better to use:

public List modifyList(List originalList) { // note - my real method uses generics
     // iterate over originalList and modify elements
     return originalList;
}

Or is it better to do the following:

public void modifyList(List originalList) {
      // iterate over originalList and modify elements
      // since java objects are handled by reference, the originalList will be modified
      // even though the originalList is not explicitly returned by the method
}

Note - The only difference between the two methods is the return type (one function returns void and the other returns a List).

Answer 1

It all depends on how you are using your List - if you are implementing some kind of list and this is the non-static method of your List class, then you should write

public List modifyList() // returning list

or

public int modifyList() // number of elements changed

If it's method outside this class

About performing operations on List or its copy: you should consider desired bahaviour and your expectations - the most importantly - do I need "old" list copy?. Deep copying list can be a little overhead. Shallow copy will unable you to perform operations on certain elements of list (i.e. changing it's attributes - if they are objects) without affecting the "old" list.

About returning void: it's good practise to return changed list (or at least number of changed elements) which will allow you to chain methods invocations, if not needed you can always ignore it.

Answer 2

If you are just manipulating the list, it entirely depends on temperament. Some people(including me) would argue is easier to read code using the first option (and it allows for chaining as pointed out by Adam, if you want that sort of thing).

However, keep in mind that its not really a reference being passed in. Its a pointer really. Hence, if you reinitialize the originalList instance for some reason, as in putting a

originalList = new ArrayList();

in your method body. This will not affect the list you actually passed into the method.

Answer 3

In my opinion you should only encourage method chaining with immutable classes.

If your function mutates an object it is too easy to do it accidentally if in a chain of methods.