Composing Methods: Remove Assignments to Parameters

Posted by Tim Koschützki, on Jul 06, 2007 - in Coding Techniques & Tools » Refactoring

When your code assigns to a parameter in a function/method, use a temporary variable instead.

Motivation

With assigning to a parameter I mean that when you pass in an object foo, you do not make it refer to a different object. Changing a parameter object is perfectly cool and I do that myself all the time. Just do not make it reference another object, like:

php
  1. function aFunction($foo) {
  2.   $foo->modify();
  3.   $foo = new SomeOtherClass;
  4. }

The reason this is so awkward is the confusion between pass by value and pass by reference in PHP. Whereas in PHP4 everything was
passed in by value as default, PHP5 passes objects around by reference. In PHP4 when you assign the parameter another object this will not be reflected in the calling routine, which can lead to quite nasty bugs if you forget to insert the by-reference operator (&).

In PHP5 this is not so much of a problem as it passes objects around by reference anyways. So if, in PHP5, you assign to a parameter you must have a good reason anyways, because you will change the parameter object also in the calling routine with your assignment. However, I find that in PHP5 there is another area of confusion making this refactoring worthwhile for both PHP4 and PHP5. That area of confusion is within the body of the code itself. It is much clearer to use only the parameter to represent what has been passed in, because that is consistent usage and therefore a good coding practice.

It will make your code much more readable and prevents by-reference confusion and therefore big problems in the future. In PHP5, assigning to an parameter object within a method will not be reflected outside the method. So I suggest you just don't do it anyways. There seems to be no point in it anyways.

Think of hairloss. ;] Haha, I seem to be bringing up hairloss all the time in connection with bug fixing. :]

Mechanics

  • Create a temporary variable for the parameter.
  • Replace all references to the parameter, made after the assignment, to the temporary variable.
  • Change the assignment to assign to the temporary variable.
  • If you are on PHP5 or PHP4 (using the by-reference operator), look in the calling method to see if the parameter is used again afterward and try to understand what is changed where in your code and document that. Make sure you only return one value from the method. If more than one call-by-reference parameter is assigned to and returned, try to use a data clump or Extract Method.

Code Example

Let's start with the following routine:

php
  1. function price($inputVal, $quantity, $anotherFactor) {
  2.   if ($inputVal > 50) $inputVal -= 30;
  3.   if ($quantity > 20) $inputVal -= 5;
  4.   if ($anotherFactor> 1000) $inputVal -= 2;
  5.   return $inputVal;
  6. }
  7.  
  8. echo price(50, 10, 1200);

Replacing with a temp leads to:

php
  1. function price($inputVal, $quantity, $anotherFactor) {
  2.   $result = $inputVal;
  3.  
  4.   if ($inputVal > 50) $result -= 30;
  5.   if ($quantity > 20) $result -= 5;
  6.   if ($anotherFactor> 1000) $result -= 2;
  7.   return $result ;
  8. }
  9.  
  10. echo price(50, 10, 1200);

An easy refactoring, yet quite cool. I do it all the time. Will you, too?