Menu
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?
524
It's considered bad practice in general, though some people overlook it as you can see in the other answers. For parameters like primitives that are directly passed in by value, there is no advantage in overriding the original variable.
Output parameters are the kind of code smell that is sometimes difficult to see, but when they crop up they can cause a moment of confusion, as they obscure the purpose of a function call, and therefore make our code less readable.
Output parameters are the parameters that are fetched from the response of a service call. These are formatted according to the attributes you configure for the output before displaying on the device. The service parameters have a scope and data type attached to them.
An out-parameter represents information that is passed from the function back to its caller. The function accomplishes that by storing a value into that parameter. Use call by reference or call by pointer for an out-parameter.
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.
73
How would you add something to the map in the second example? I think it would be bad practice if you have to pass an empty map that gets filled in addTokenErrorsToReport. But in this case: no, I don't think it's bad practice. How would you implement otherwise if you have several List<String> tokens that you want to process? I think the first example is the straightforward one.
2
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
