19

Suppose I am writing a method foo(int i) in Java.
Since i is passed by value it is safe to change it in foo. For example

void foo(int i) {
   i = i + 1; // change i
   ...
}

Is it considered good or bad practice to change arguments of methods in Java?

Michael
  • 41,026
  • 70
  • 193
  • 341

6 Answers6

25

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. In this case you should make a copy as suggested by @João.

For parameters whose reference is passed in by value (objects), if you modify the handle to point to a different object, that is downright confusing. It's all the more important because modifying the contents of an object passed as a parameter will modify the original object too.

If you replace the object referred to by the handle, and then modify its contents, the object referred to by the original reference in the caller will not be replaced, but someone reading the code might expect it to be.

Whereas if you don't replace the object, and modify the contents, the method calling your method might not expect this change. This category generally comes under security-related bad practices.

user207421
  • 305,947
  • 44
  • 307
  • 483
Rajesh J Advani
  • 5,585
  • 2
  • 23
  • 35
  • Everything is passed by value in Java. Objects are not "passed"; objects are not values. – newacct Aug 19 '12 at 01:42
  • Would you feel better if I used the terms call-by-value and call-by-reference instead? Also, what does "objects are not values" mean? – Rajesh J Advani Aug 19 '12 at 02:49
  • 3
    No. It is always pass-by-value (or "call-by-value"). The only types in Java are primitive types and reference types. Hence the only values are primitives and references, both of which are passed by value. Unlike C++, you cannot have a variable whose value "is" an object; you can only have a variable whose value is a reference that points to an object. Everything you do with objects must be done through a reference -- `new SomeClass()` evaluates to a reference; field and method access use references. That's what "objects are not values" means. You cannot "pass" an object. – newacct Aug 19 '12 at 08:42
  • 4
    Yes, I read all the arguments on the topic though I still think your choice of phrase is more confusing than helpful. I reworded the answer so let me know if it looks OK now. – Rajesh J Advani Aug 19 '12 at 11:53
8

It's merely a personal opinion, but I think it can be confusing for other people that may want to use the original parameter value later in that code, and may not notice that it has already been changed.

Plus, it's cheap to simply create another variable and assign it the modified value (i.e., int j = i + 1).

João Silva
  • 89,303
  • 29
  • 152
  • 158
  • 3
    My personal opinion differs from yours. Not understanding a method you are making changes to is bad. – Captain Giraffe Aug 18 '12 at 15:58
  • 4
    @CaptainGiraffe True. But shenanigans makes understanding the method harder than it has to be, and making a distinction between passed-in and (for whatever reason) changed value *can* aid understanding. –  Aug 18 '12 at 16:03
  • 1
    Every opinion in the world is always a personal opinion or a corporate opinion :) The question obviously meant to ask what the general consensus is. And that is that it's a bad idea. For the reasons stated in my answer. – Rajesh J Advani Aug 18 '12 at 16:07
3

Since i is passed by value it is safe to change it foo()

It is absolutely safe even when you pass an object reference because they are local: i.e., assigning a new reference to the local reference will not have any impact on the original reference in the calling code

It's your personal choice. However i would not change the argument values as one might loose the track of the actual value passed in to this method.

user207421
  • 305,947
  • 44
  • 307
  • 483
srikanth yaradla
  • 1,225
  • 10
  • 13
2

What is important to note is that i = i + 1; does not really change i. It only changes your local copy of i (in other words, the i in the calling code won't change).

Based on that, it is a matter of readability and avoiding unexpected behaviour in your code by complying with the POLS (Principle Of Least Surprise).

assylias
  • 321,522
  • 82
  • 660
  • 783
  • It changes the local `i` but not the original value passed as the parameter, which doesn't have to be called `i` and indeed doesn't even have to be a variable. – user207421 Nov 07 '19 at 10:02
1

Neutral. But it would be considered a better practice by many people to change the method to:

void foo(final int i) {
    int j = i + 1; // not change i
    ...
}

Feel free to work either way.

fdreger
  • 12,264
  • 1
  • 36
  • 42
1

Depends on context. I lean towards "bad practice", for two reasons:

  1. Some people may think the original value is being changed.
  2. It may make the code harder to reason about (mitigated with appropriately-short methods).

A third issue pops up when it's a reference value. If you modify the parameter reference to point at something else and change its state, the original won't be modified–this may or may not be what's intended. If you create another reference to the parameter and change the new reference's state, the parameter's reference will be changed–which also may or may not be what's intended.

Dave Newton
  • 158,873
  • 26
  • 254
  • 302
  • 1
    #1 is not a good reason. If they don't know what argument passing style a method is using, they have worse problems than misunderstanding that one method and should learn that first before doing any more coding. –  Aug 18 '12 at 16:04
  • 1
    @delnan It's a fine reason--any obfuscation should be avoided when it can be, which it can be in this situation. You may not *agree* with the reason, which is fine. – Dave Newton Aug 18 '12 at 16:32
  • Okay, I disagree with the reason. But not because I don't eschew obfuscation, but because I think the meaning it as obvious as the meaning of `a + 1` to everyone who actually knows the language. –  Aug 18 '12 at 16:36