Why is my method changing the value of my original variable?

Question

I have created a method that takes a List as an input, brewMethod.getmMethodBrewPours() and depending on the users preference, the method returns those variables as they are or multiplied by a number. I store the returned value as a new variable, brewPours. The issue is, not only is this method returning new values to be stored, but it's also changing the values of the original variable. Why is this being changed, and what should I do to prevent it?

Code that calls method:

// Custom class storing sets of data
        brewMethod = i.getParcelableExtra("brew_method");

        //logs original packaged values
        for (int j = 0; j < brewMethod.getmMethodBrewPours().size(); j++) {
            Log.v("Original pour value #: " + j + ": ", brewMethod.getmMethodBrewPours() .get(j).toString());
        }

        //changes values based on users preference
        brewPours = replacePours(brewMethod.getmMethodBrewPours());

        //report the values again of the original packed values, but they are changed for some reason
        for (int k = 0; k < brewMethod.getmMethodBrewPours().size(); k++) {
            Log.v("New value on Original pour value #: " + k + ": ", brewMethod.getmMethodBrewPours() .get(k).toString());
        }

        //log the values of the new variable brewPours
        for (int l = 0; l < brewPours.size(); l++) {
            Log.v("Value for new variable brewPours pour # " + l + ": ", brewPours.get(l).toString());
        }

Code of the method:

    public List<Integer> replacePours(List<Integer> waterPours) {

    SharedPreferences pref = getSharedPreferences("preferences", MODE_PRIVATE);

    Integer newServingSize = pref.getInt("pref_key_serving_size", 1);

    if (newServingSize == 2) {
        for (int i = 0; i < waterPours.size(); i++) {
            newPour = waterPours.get(i) * 2;
            waterPours.set(i, newPour);
        }
    }

    return waterPours;
}

Output:

06-19 16:50:34.474 30159-30159/com.brewguide.android.coffeebrewguide V/Original pour value #: 0:: 60
06-19 16:50:34.474 30159-30159/com.brewguide.android.coffeebrewguide V/Original pour value #: 1:: 200
06-19 16:50:34.474 30159-30159/com.brewguide.android.coffeebrewguide V/Original pour value #: 2:: 300
06-19 16:50:34.474 30159-30159/com.brewguide.android.coffeebrewguide V/Original pour value #: 3:: 380
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/New value on Original pour value #: 0:: 120
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/New value on Original pour value #: 1:: 400
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/New value on Original pour value #: 2:: 600
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/New value on Original pour value #: 3:: 760
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/Value for new variable brewPours pour # 0:: 120
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/Value for new variable brewPours pour # 1:: 400
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/Value for new variable brewPours pour # 2:: 600
06-19 16:50:34.475 30159-30159/com.brewguide.android.coffeebrewguide V/Value for new variable brewPours pour # 3:: 760

Edit: I adjusted the method's code following the advice from @Vampire and got the results I was looking for. Below is my adjusted code, thanks!

public List<Integer> replacePours(List<Integer> waterPours) {

    List<Integer> returnedPours = new ArrayList<>();
    SharedPreferences pref = getSharedPreferences("preferences", MODE_PRIVATE);

    Integer newServingSize = pref.getInt("pref_key_serving_size", 1);

    if (newServingSize == 2) {
        for (int i = 0; i < waterPours.size(); i++) {
            int newPour = waterPours.get(i) * 2;
            returnedPours.add(newPour);
        }
    }
    return returnedPours;
}

It is changing the original values because that is what the code does: you are iterating over `waterPours` and at the same time you are modifying it with `waterPours.set(i, newPour);`. If you wish *not* to modify it you should add the `newPour`s in a new list. Also be carefull when iterating and modifying the same list, as you can get a `ConcurrentModificationException` — DarkCygnus, Jun 19 '17 at 23:59
Where is `newPour` defined? It should be defined locally to the loop, but isn't, and where it is defined you don't show. — Lew Bloch, Jun 20 '17 at 00:41

Vampire · Accepted Answer · 2017-06-20T00:05:19.757

4

The original List is modified because you modify it. Java is call-by-value, but the value you are giving to the method is a reference to an object, not the object. That means you are not giving the list to the method and getting a list out, but you are giving the reference to a list into the method. As you modify the referenced object, you modify the original object. You have to create a new list in your method and return that instead of modifying the list you gave to it.

edited Jun 20 '17 at 00:05

answered Jun 19 '17 at 23:57

Vampire

35,631
4
76
102

2

FWIW - https://stackoverflow.com/questions/40480/is-java-pass-by-reference-or-pass-by-value – Oliver Charlesworth Jun 19 '17 at 23:59

Shaishav Jogani · Answer 2 · 2017-06-20T01:05:08.373

2

Understand the difference between Call-by-reference and call-by-value.

In JAVA, when you pass the object in function it pass its reference to the function. So, any change on the object in function reflects the change outside the function.

In your case you are changing the value of original object by this line : waterPours.set(i, newPour);

So, it's better if you create new object in function and modified it.

public List<Integer> replacePours(List<Integer> waterPours) {

    List<Integer> tempList = new ArrayList<Integer>(waterPours);

    SharedPreferences pref = getSharedPreferences("preferences", MODE_PRIVATE);

    Integer newServingSize = pref.getInt("pref_key_serving_size", 1);

    if (newServingSize == 2) {
        for (int i = 0; i < tempList.size(); i++) {
            int newPour = tempList.get(i) * 2;
            tempList.set(i, newPour);
        }
    }

    return tempList;
}

edited Jun 20 '17 at 01:05

answered Jun 20 '17 at 00:07

Shaishav Jogani

2,111
3
23
33

1

This is not really about a difference between call-by-ref and call-by-value, given that Java is call-by-value. – Oliver Charlesworth Jun 20 '17 at 00:10
You should have shown the declaration of `newPour` moved to the correct location, inside the loop. – Lew Bloch Jun 20 '17 at 00:42
@LewBloch Done. Though, I thought OP didn't declare the `newPour` so, maybe it is some global variable. – Shaishav Jogani Jun 20 '17 at 01:06

Why is my method changing the value of my original variable?

2 Answers2