Manipulating Objects in Methods instead of returning new Objects?

Let’s say I have a method that populates a list with some kind of objects. What are the advantages and disadvantages of following method designs?

void populate (ArrayList<String> list, other parameters ...)

ArrayList<String> populate(other parameters ...)

Which one I should prefer?

This looks like a general issue about m开发者_运维问答ethod design but I couldn't find a satisfying answer on google, probably for not using the right keywords.

The second one seems more functional and thread safe to me. I'd prefer it in most cases. (Like every rule, there are exceptions.)

The owner of the populate method could return an immutable List (why ArrayList?).

It's also thread safe if there is no state modified in the populate method. Only passed in parameters are used, and these can also be immutable.

Other than what @duffymo mentioned, the second one is easier to understand, thus use: it is obvious what its input and output is.

Advantages to the in-out parameter:

• You don't have to create as many objects. In languages like C or C++, where allocation and deallocation can be expensive, that can be a plus. In Java/C#, not so much -- GC makes allocation cheap and deallocation all but invisible, so creating objects isn't as big a deal. (You still shouldn't create them willy-nilly, but if you need one, the overhead isn't as bad as in some manual-allocation languages.)
• You get to specify the type of the list. Potential plus if you need to pass that array to some other code you don't control later.

Disadvantages:

• Readability issues.

  • In almost all languages that support function arguments, the first case is assumed to mean "do something with the entries in this list". Modifying args violates the Priciple of Least Astonishment. The second is assumed to mean "give me a list of stuff", which is what you're after.
  • Every time you say "ArrayList", or even "List", you take away a bit of flexibility. You add some overhead to your API. What if i don't want to create an ArrayList before calling your method? I shouldn't have to, if the method's whole purpose in life is to return me some entries. That's the API's job.

• Encapsulation issues:

  • The method being passed a list to fill can't assume anything about that list (even that it's a list at all; it could be null).
  • The method passing the list can't guarantee anything about what the method does with it. If it's working correctly, sure, the API docs can say "this method won't destroy existing entries". But considering the chance of bugs, that may not be worth trusting. At least if the method returns its own list, the caller doesn't have to worry about what was in it before. And it doesn't have to worry about a bug from a thousand miles away corrupting data it should never have affected.

• Thread safety issues.

  • The list could be locked by another thread, meaning if we try and lock on it now it could potentially lock up the app.
  • Or, if not locked, it could still be modified by another thread, in which case we're no less screwed. Unless you're going to write extra code to handle concurrent-modification exceptions everywhere.
  • Returning a new list means every call to the method can have its own list. No thread can mess with another thread's return value, unless the class is very badly designed.

Side point: Being able to specify the type of the list often leads to dependencies on the type of the list. Notice how you're passing ArrayLists around everywhere. You're painting yourself into corners by saying "This is an ArrayList" when you don't need to, but when you're passing it to a dozen methods, that's a dozen methods you'll have to change. (Not entirely related, but only slightly tangential. You could change the types to List rather than ArrayList and get rid of this. But the more you're passing that list around, the more places you'll need to change.)

Short version: Unless you have a damn good reason, use the first syntax only if you're using the existing contents of the list in your method. IE: if you're modifying it, or doing something with the existing values. If you intend to return a list of entries, then return a List of entries.

The second method is the preferred way for many reasons. primarily because the function signature is more clear and shows what its intentions are.

It is actually recommended that you NEVER change the value of a parameter that is passed in to a function unless you explicitly mark it as an "out" parameter.

it will also be easier to use in expressions

and it will be easier to change in the future. including taking it to a more functional approach (for threading, etc.) if you would like to