Does Modifying an Incoming Parameter Count as a Code Smell?

Written by Kristijan Kralj in Software Engineering

Modifying the value of an incoming method parameter is a common programming practice to handle various scenarios. But when doing that, you need to be careful. You don’t want to break the behavior of other parts of your program just because the parameter changed its value in your current method.

As a general rule, modifying an incoming method parameter is not a good practice. Reasons why you should avoid it are:

  • It can change the variables’ original value,
  • It reduces the code’s readability and maintainability,
  • Multiple assignments make it difficult to track the parameter’s exact value.

Let’s dive deeper and understand why you should stop doing this and how you can refactor such methods.

Is modifying an incoming parameter a bad practice?

Sometimes you may have seen programs with methods that modify incoming parameters by reassigning new values. It may not seem an issue at first glance. However, there a several good reasons to avoid this practice.

It can change the variables’ original value

You can pass parameters to a method either by its reference or value.

First, if you pass a parameter as a reference to a method and if your method changes its value, it will return the updated value to the calling code. Unless your program flow uses the updated value for the rest of the logic, it can have serious issues if the program unknowingly modifies the incoming parameters inside methods.

Lets’ see its effect using the following code example.

using System;
 
namespace Example
{
     class Program
     {
         static void Main(string[] args)
         {
             double initialDeposit = 100.0;
             Console.WriteLine("initial deposit: {0}", initialDeposit);
             double finalDeposit = CalculateFinalDeposit(ref initialDeposit);
             Console.WriteLine("final deposit: {0}", finalDeposit);
             Console.WriteLine("initial deposit after: {0}", initialDeposit);
             //rest of the logic using the initial deposit    
         }
         public static double CalculateFinalDeposit(ref double deposit)
         {
              deposit = deposit  + (deposit * 2.5/100);
              return deposit;
         }
     }
}

Output:

initial deposit: 100
final deposit: 102.5
initial deposit after: 102.5

In the above program, the method calculateFinalDeposit takes the initialDeposit as an incoming parameter passed as a reference. Then the method modifies the parameter itself and returns it. The rest of the program uses the modified value for initialDeposit.

It would provide incorrect outputs if you did not notice that its original value has changed and intend to use its original initial value throughout the program.

It reduces the code’s readability and maintainability

In most codes, you will see methods with parameters passed by value. In that case, reassigning a value to the incoming parameters will not change its initial value. But the problem lies with how it impacts the codes’ readability and maintainability in the future.

This reassignment confuses other programmers and makes it harder to change the code. Also, there is no special advantage you gain from it.

For example, the following method takes an object as an incoming parameter and changes its instance variable value. It does not change its initial value, so it will not be a problem for now. However, anyone new to the code may think the method needs to change the parameter’s initial value.

If you try to modify such code, you will have to take extra work to review the whole code to ensure you do not break anything.

public void convertObjValues(Sample s1, int number)
{
      if (s1.value < 0) {
        return;
      }
      s1.value = s1.value + number;
}

Multiple assignments make it difficult to track the parameter’s exact value

The problem worsens if the method modifies the parameter by assigning different values at different times. Also, the method may modify multiple parameters. Such multiple modifications make it harder for code readers to guess what exactly it should contain at a given time. If the method contains multiple loops and conditions, tracking down the parameters’ lifecycle is challenging.

Thus, modifying incoming parameters is a bad practice you should avoid.

How to remove assignments to parameters?

There are two ways you can avoid modifying incoming parameters.

#1 Use “Remove assignments to parameters” refactoring

In this approach, you create a local variable inside the method and first assign the initial parameter value to it before using it. The rest of the code uses this local variable to do the required modifications. In this way, the original parameter can keep its value intact.

The following code illustrates this simple refactoring. The incoming parameter value is now assigned to the local variable result, and the method modifies that variable throughout it. This assignment now does not confuse the reader because it only changes within the method scope.

public static int CountValue(int value)
{      
    int result = value;
    if (result < 0) {
        return;
    }   
    result = result + 10;
    return result;
}

#2 Use “Split temporary variable” refactoring

You can use the “Split temporary variable” and “Remove assignments to parameters” refactoring if a method is long.

public static double CalculateValue(double amount, string type)
{
    amount = amount + 100;
    if (type == 'deposit')
    {
        amount = amount + (amount * 2.5 / 100);
        return amount;
    }
    else if (type == 'withdraw')
    {
        amount = amount - (amount * 0.5 / 100);
        return amount;
    }

    //......
}

The above code changes the incoming parameter amount multiple times by assigning different unrelated values. Here, the parameter itself has become a temporary variable. If you need to modify the code, you must check each assignment to ensure it returns the correct value.

To avoid such complications, replace the parameters of each assignment with a new variable. It would help if you renamed the variable with a name that reflects the assigned value.

The following code now contains separate variables for each assignment, improving the code readability and maintenance.

public static double CalculateFinalDeposit(double amount, string type)
{
    double adjustedAmount = amount + 100;

    if (type == 'deposit')
    {
        double deposit = adjustedAmount + (adjustedAmount * 2.5 / 100);
        return deposit;
    }
    else if (type == 'withdraw')
    {
        double withdraw = adjustedAmount - (adjustedAmount * 0.5 / 100);
        return withdraw;
    }

    //......
}

Conclusion

Modifying incoming parameters inside a method is a bad code practice you should always avoid. The first reason to avoid this mistake is that it can change the original variable values. Using it throughout the rest of the code without being aware of it will cause bugs in the program. Secondly, it impacts the code readability and maintenance in the future.

If the method is too large and contains multiple assignments to the parameter, it is not easier to track the exact parameter value. You can avoid this by using the simple refactoring techniques described above.

Recent Posts