Is using output parameters considered bad practice?

coding-style, java

Solution

As with most things, it's only "bad practice" if it leads to poorly functioning / unreadable / hard-to-maintain code or if you don't know why you're doing it.

In most cases using an output parameter doesn't have those effects.

In your addTokenErrorsToReport, it certainly is an appropriate approach. You are adding token errors to a report - the function needs to know the tokens it is adding and the report it is adding to. The function clearly performs precisely the operation it was designed to perform with no disadvantages.

If you were to take the createTokenErrorsReport approach, you would have to follow every call to it by inserting the new tokens in the existing report. If adding tokens to an existing report is a common operation, it most definitely makes sense to have a method that adds. That's not to say that createTokenErrorsReport shouldn't exist as well - if creating new reports from a token list is a common operation, then you would want a function that does that.

A great example of a good use of an output parameter is `Collections.sort`, which sorts a list in place. The performance hit of creating a new copy of the list and returning the sorted copy is avoided, while at the same time it does not limit you from creating a copy and sorting the copy if you want to.

Just use the best tool for the job and keep your code succinct.

Problem

Every time I see a method were one of the parameters is an output parameter like ``` void addTokenErrorsToReport(List<String> tokens, Map<String, Integer> report) ``` I get the feeling that this is just plain wrong. From my point of view, parameters in general should be immutable, and not changed within a method. E.g., the above method could be rewritten to ``` Map<String, Integer> createTokenErrorsReport(List<String tokens) ``` The returned `Map` could then be merged with the original report Map. Is this assumption right? Or are both versions equally acceptable?

Original source