Is it problematic to assign a new value to a method parameter?

Question

Eclipse has an option to warn on assignment to a method's parameter (inside the method), as in:

public void doFoo(int a){
   if (a<0){
      a=0; // this will generate a warning
   }
   // do stuff
}

Normally I try to activate (and heed) almost all available compiler warnings, but in this case I'm not really sure whether it's worth it.

I see legitimate cases for changing a parameter in a method (e.g.: Allowing a parameter to be "unset" (e.g. null) and automatically substituting a default value), but few situations where it would cause problems, except that it might be a bit confusing to reassign a parameter in the middle of the method.

Do you use such warnings? Why / why not?

Note:

Avoiding this warning is of course equivalent to making the method parameter final (only then it's a compiler error :-)). So this question Why should I use the keyword "final" on a method parameter in Java? might be related.

Answer 1

The confusing-part is the reason for the warning. If you reassign a parameter a new value in the method (probably conditional), then it is not clear, what a is. That's why it is seen as good style, to leave method-params unchanged.

Answer 2

For me, as long as you do it early and clearly, it's fine. As you say, doing it buried deep in four conditionals half-way into a 30-line function is less than ideal.

You also obviously have to be careful when doing this with object references, since calling methods on the object you were given may change its state and communicate information back to the caller, but of course if you've subbed in your own placeholder, that information is not communicated.

The flip side is that declaring a new variable and assigning the argument (or a default if argument needs defaulting) to it may well be clearer, and will almost certainly not be less efficient -- any decent compiler (whether the primary compiler or a JIT) will optimize it out when feasible.

Answer 3

Reassigning to the method parameter variable is usually a mistake if the parameter is a reference type.

Consider the following code:

MyObject myObject = new myObject();
myObject.Foo = "foo";
doFoo(myObject);

// what's the value of myObject.Foo here?

public void doFoo(MyObject myFoo){   
   myFoo = new MyObject("Bar");
}

Many people will expect that at after the call to doFoo, myObject.Foo will equal "Bar". Of course, it won't - because Java is not pass by reference, but pass by reference value - that is to say, a copy of the reference is passed to the method. Reassigning to that copy only has an effect in the local scope, and not at the callsite. This is one of the most commonly misunderstood concepts.

Answer 4

Assigning a method parameter is not something most people expect to happen in most methods. Since we read the code with the assumption that parameter values are fixed, an assignment is usually considered poor practice, if only by convention and the principle of least astonishment.

There are always alternatives to assigning method parameters: usually a local temporary copy is just fine. But generally, if you find you need to control the logic of your function through parameter reassignment, it could benefit from refactoring into smaller methods.

Answer 5

Different compiler warnings can be appropriate for different situations. Sure, some are applicable to most or all situations, but this does not seem to be one of them.

I would think of this particular warning as the compiler giving you the option to be warned about a method parameter being reassigned when you need it, rather than a rule that method parameters should not be reassigned. Your example constitutes a perfectly valid case for it.

Answer 6

I sometimes use it in situations like these:

void countdown(int n)
{
   for (; n > 0; n--) {
      // do something
   }
}

to avoid introducing a variable i in the for loop. Typically I only use these kind of 'tricks' in very short functions.

Personally I very much dislike 'correcting' parameters inside a function this way. I prefer to catch these by asserts and make sure that the contract is right.

Answer 7

I usually don't need to assign new values to method parameters.

As to best-practices - the warning also avoids confusion when facing code like:

       public void foo() {
           int a = 1;
           bar(a);
           System.out.println(a);
       }

       public void bar(int a) {
           a++;
       }

Answer 8

You shoud write code with no side effect : every method shoud be a function that doesn't change . Otherwise it's a command and it can be dangerous.

See definitions for command and function on the DDD website :

Function : An operation that computes and returns a result without observable side effects.

Command : An operation that effects some change to the system (for example, setting a variable). An operation that intentionally creates a side effect.