users
[Top] [All Lists]

IDE Warning - Parameter value should not be assigned - Update

To: "users@xxxxxxxxxx" <users@xxxxxxxxxx>
Subject: IDE Warning - Parameter value should not be assigned - Update
From: "Edward Sumerfield" <esumerfd@xxxxxxxxx>
Date: Thu, 3 Aug 2006 08:57:43 -0400
Delivered-to: mailing list users@cinjug.org
Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:reply-to:to:subject:mime-version:content-type; b=Coh0lYyKH+zfeEQjP5PQ8CdqqZdFcb6MZWEliJXBLfi8FhckUjWilyV3yfY0aOL6qwuoEk8lwbTdHRjrjHY6MUOk3bzkO0iPBl3iandj8whnd8Z+/Q56FZVuEXDhl4glHYHrJbvA6FwApf4WljtWnNw3BEUX7tMi6q0kvPX8Uko=
Mailing-list: contact users-help@cinjug.org; run by ezmlm
Reply-to: esumerfd@xxxxxxxxxxxxxx
I came up with another reason to drop the rule. There is a bug in the following code but can really tell by looking at it. One would presume that the altered value should be passed to the method instead of the parameter. If the new value was distincly different then it should have a different name but if we are just trying to avoid the compiler warnings we are likley to keep them the same.

    public void foo(int a_value)
    {
        int value = a_value;
        value++;
        doSomething(a_value);
        doSomethingElse(value);
    }

Not sure if anyone read my blog entry on pre-bugs but this is a similar case. Ressigning parameters to local variables increases the risk of a bug and hence is a pre-bug which should be avoided.

    http://esumerfield.blogspot.com/2005/09/pre-bug-variable-notation.html


---------- Forwarded message ----------
From: Edward Sumerfield <esumerfd@xxxxxxxxx>
Date: Aug 2, 2006 1:54 PM
Subject: Re: [cinjug-users] IDE Warning - Parameter value should not be assigned
To: "users@xxxxxxxxxx" <users@xxxxxxxxxx>

In JDK1.5 the warning will say that newList "can only be null" which should catch that scenario.

Once we accept that the changed value can not be returned we are left with a "read only" value, expecially if we add James' final operator.

So the resulting code is just redundent.

    public void foo(final int a_value)
    {
        int value = a_value;
        value++;
        doSomething(value);
        doSomethingElse(value);
    }

To Brians' point, I agree that in most cases this problem doesn't come up and as methods are reduced in size the problem goes away. However, with short methods the "hard to follow" concern goes away aswell. There is also the counter argument that two variables is are complex than one. The hard to follow concern would be solved with good naming conventions.

    public void foo(int a_numberOfRulesToBreak)
    {
        a_numberOfRulesToBreak++;
        doSomething(a_numberOfRulesToBreak);
        doSomethingElse(a_numberOfRulesToBreak);
    }

I think, given the severity of the problem, that this warning should now be ignored. Less code is better.

This means I am agreeing with Rob the Ruby guy. See you at NoFluff dude. :-)



On 8/2/06, Joe Fox <joe.fox@xxxxxxxxxxxx> wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yikes, be careful here! The rule still applies, but for maybe different
reasons.

In the example, both changed values in getFoo will be LOST.  Neither
value will be changed from the perspective of the code calling this
method.  As long as you simply use those values within the scope of the
method, you'll be okay.  If you expect to get a value back, you'll be in
for a surprise.

The int is passed by value, so the a_value that you're modifying is a
copy on the stack, and the stack frame for this method is lost when the
method returns and any changes to that int are lost with it.  The Object
ref is also copied onto the stack, so when the method returns, the
reference to the new Object is lost, and with no reference to the object
the new Object will be garbage collected.

Another good (non-compiling semi-pseudo code) example:

public void someMethod() {
        List newList = null;

        initializeList(newList);

        newList.get(index);  // OOPS!  Null pointer!
}

public void initializeList(List newList) {
        newList = new ArrayList();
        ... set some values ...
}

A proper compiler error may catch this error.

~Joe

Edward Sumerfield wrote:
> Is the warning "The parameter value should not be assigned" still valid in
> Java?
>
>    public void getFoo(int a_value, Object a_object)
>    {
>        a_value += 1;                     // WARNING
>        a_object = new Object();     // WARNING
>    }
>
> It has always been good programming practice to avoid assigning parameters
> because in C/C++ you might be setting a pointer that the caller would not
> know had changed.
>
> In java it is not possible set the object references that are passed in and
> the primitive types are passed by value so what is the danger?
>
> I propose that the old bad practive rule be revoked in the light of better
> languages?
>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFE0Ndnx1T2M+8Q7OYRAhIbAJ9Rn2Z63b1y+DNxfp4Nac/SbG1ycwCdHqNv
PzlnsS0Q/PunvVOiJaYIo0U=
=9ocX
-----END PGP SIGNATURE-----


--
Ed


--
Ed
<Prev in Thread] Current Thread [Next in Thread>
  • IDE Warning - Parameter value should not be assigned - Update, Edward Sumerfield <=