The parameter 'foo' should not be assigned -- what's the harm?

Question

Compare this method:

void doStuff(String val) {     if (val == null) {         val = DEFAULT_VALUE;     }      // lots of complex processing on val } 

... to this method:

void doStuff(String origVal) {     String val = origVal;     if (val == null) {         val = DEFAULT_VALUE;     }      // lots of complex processing on val } 

For the former method, Eclipse emits the warning "The parameter 'val' should not be assigned". Why?

To my eye, the former is cleaner. For one thing, it doesn't force me to come up with two good names for val (coming up with one good one is hard enough).

(Note: Assume there is no field named val in the enclosing class.)

Answer 1

It doesn't look like anyone's made the curmudgeon's case here.

I would generally not mutate parameters, and in fact I tend to mark my parameters final in order to explicitly forbid it. A few reasons:

• Assignment to a parameter could be confused with an attempt to use it as an "output parameter", ref: javapractices.com, and clarity is everything

• Favor Immutability, and that goes for parameter values as much as anything else. Primitives are just a degenerate case of the same thing, it's (generally) easier to reason about immutable variables. Ref, Effective Java Item 13, or javapractices.com

• And finally (NPI), Use final liberally, javapractices.com. However ugly it may be in parameter signatures, I believe it tends to identify unexpected errors and it highlights mutable variables, which generally should be the exception. Most mutable variables in most code are there either for laziness or a perception that it has some effect on performance, when judiciously chosen, immutable, and well-named intermediate computations are clearer, easier to read and verify, and can be cleanly optimized for performance with no help from you.

I can't speak intelligently to your specific case in the abstract, but barring all the other things I might do differently, I'd favor:

void doStuff(final String origVal) {     final String valOrDefault = (origVal == null) ? DEFAULT_VALUE : origVal;     //lots of complex processing on valOrDefault  } 

or even (assuming you wouldn't cope with a null value in a real method with only one argument, it must be part of something more complex)... Also, in general, methods which accept null as a parameter should be explicitly documented as doing so, if only to reinforce the assumption that null parameters ought to be the exception. In the second method, you might even make use of the @NonNull annotation.

/**   * @param origVal string giving value, possibly null, in which case DEFAULT_VALUE is assigned   */ void doStuff(final String origVal, ... ) {     final String valOrDefault = (origVal == null) ? DEFAULT_VALUE : origVal;      // similar mucking about to make all the parameters behave, separate from     // actually operating on them...     ...     reallyDoStuff(valOrDefault,...); }  private void reallyDoStuff(final String value, ...) {    assert (value != null);    // do your complex processing } 

Related questions (and associated argument) on StackOverflow: "Using final modifier whenever applicable in Java...", "final keyword in method parameters", "Do you final-ize local variables and method parameters in Java".