11

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).

4
  • This is largely a matter of opinion. On the one hand returning the list can allow method chaining. But it also confuses the method signature. Somewhat related: Benefits and drawbacks of method chaining and a possibility to replace all void return parameters by the object itself Nov 20, 2013 at 15:25
  • 1
    @RichardTingle - I will agree it is somewhat subjective, but I was wondering if there was a good argument against one option or the other that might make my code less maintainable in the future.
    – David
    Nov 20, 2013 at 15:26
  • Do you want to method chain? If not then the second is definately better as it doesn't make it look like you're using the originalList purely as an input rather than modifying it Nov 20, 2013 at 15:26
  • This question is pure opinion until you define "better." Nov 20, 2013 at 20:09

5 Answers 5

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.

1

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.

1

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.

1

One possible benefit of Option 1 is that it can accept a null List. For example, if you are collecting Foos, and generally create a brand new List, but want the option to add to an existing list. e.g. (note name of method as well)

public List<Foo> appendFoos(List<Foo> in) {
  if (in == null)
    in = new ArrayList<Foo>;

  // now go do it, e.g.
  in.add(someFooIFound);

  return in;
}

and, if you wish, add an explicit no-arg "get" method as well

public List<Foo> getFoos() {
   return appendFoos(null);
}

Now, in Option #2, you could do this by having the user create a new, empty ArrayList and passing that in, but Option #1 is more convenient. i.e.

Option 1 Usage:

List<Foo> theFoos = getFoos();

Option 2 Usage:

List<Foo> theFoos = new ArrayList<Foo>();
appendFoos(theFoos);
0

As List is mutable, so second method is better. You don't need to return modified List.

1
  • Not if you decorate it with Collections#unmodifiableList(List). Nov 20, 2013 at 15:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.