0

I am currently working with an "interesting" code-base and see the following type of thing alot in the code.

public Object doSomething()
{
    Object obj = new Object();

    // Do some stuff to the object

    obj = doSomthingElse(obj);    

    return obj;
}

I always feel when looking at this kind of code that it is somewhat incorrect to have an object be passed into a method as a parameter but also set the object returned by the method to the same variable that was referencing the passed object. I would usually do something like the following, where I create a new variable to avoid any confusion.

public Object doSomething()
{
    Object obj = new Object();

    // Do some stuff to the object

    Object anotherObj = doSomthingElse(obj);     

    return anotherObj;
}

Am I wrong in thinking the second code snippet is more readable/correct?

| improve this question | |
  • 1
    Do you use Object to mean that different methods handle different classes but display the same pattern, or does your code base use the actual Object class a lot? – Simon Apr 3 '14 at 11:37
  • I can't see that this could have any answer than just one opinion against another. The above code would not attract my attention either way. – david.pfx Apr 3 '14 at 12:46
  • The former code has an implication to me that "obj" is an object (probably immutable) for which a replacement (probably a copy with some details changed) is being returned. The latter code implies that the returned object is not closely related to the one passed in (although appropriate variable naming could change my feelings about that). – Jules Apr 3 '14 at 13:37
  • Would you feel differently if the variable was a primitive value instead of an object reference? For example, if your function had a local variable, double d; and it assigned d = f(d, ...)? – Solomon Slow Aug 7 '17 at 16:17
  • Possible duplicate of How would you know if you've written readable and easily maintainable code? – gnat Aug 7 '17 at 16:47
3

If the method call is transforming the object without creating a copy of it (and then returning the same object, with the changes), the statement obj = doSomthingElse(obj); has no effect (you could just perform doSomthingElse(obj); instead).

If the method creates and returns another object (that is a modified copy of the original), then what the statement obj = doSomthingElse(obj); does is called variable reuse. To say that it is a good or a bad practice would be nothing more than an opinion.

Sticking to your abstract example, my opinion would be that you need to add some comments to the code explaining what the obj variable represents. Then, reusing it or not, you should try to stick to what is documented (the variable's role in the code).

If the result of doSomthingElse(obj) is another object and represents something different from the documented role of the variable obj, then you should use another variable (as you did), IMHO.

| improve this answer | |
  • 1
    If you need to add comments to explain the "obj" variable, you should give it a better name to begin with – user949300 Aug 8 '17 at 0:44
  • Yes. Trying to make the code self-explanatory should be the first option. It's what I think nowadays (the question is more than 3 years old :)). – igorrs Aug 8 '17 at 13:00
1

There's no right or wrong answer to this. It's very subjective. I would say that the style you are leaning towards is widely considered valid but maybe a little picky to some.

If you don't want to reuse variables, I would strongly recommend declaring them final. This will make your intention clear to the compiler as well as other devs.

I would avoid declaring a variable simply to return it but that's probably just there to illustrate your scenario.

| improve this answer | |
  • Agree that Declaring avariable just to return is debateable. However, it aids in debugging, and is a good place to explain what is happening bygiving it agood variable name. Of course, the function name should also be explanatory too. – user949300 Aug 8 '17 at 0:52
1

Variable reuse isn't horribly wrong, but you miss a chance at writing clearer code by giving variables good names. For example, if you were manipulating a Point, use

Point origin = new Point();
Point translated = doSomething(origin);
return translated;
| improve this answer | |