Refactor by Reference

Fast eine Stunde Fehlersuche hat mir am Freitag eine schöne Eigenart von PHP gekostet, mit der ich nicht gerechnet hatte und deswegen möchte ich sie mit euch teilen, denn ich denke, dass jeder in diese Lage kommen kann.

Wie der Titel schon beschreibt wollte ich eine Funktion verändern, also Refactoring betreiben. Eigentlich ging es nur darum eine Funktion aus einer Klasse in eine andere zu verschieben. Um sicherzustellen, dass danach alles noch funktioniert, wie es sollte, habe ich die alte Funktion weiterhin aufrufbar gelassen, habe sie aber einfach auf die neue umgeleitet. Im Großen und Ganzen sah die Ursprungsklasse wie folgt aus:

<?php
  class helperClass
  {
    public static function trimArray( $array )
    {
      // lösche alle leeren Elemente aus dem Array
      return $array;
    }
  }
?>

Also kein Hexenwerk diese einfache Funktion zu verschieben. Gesagt getan habe ich dann eine neue Klasse implementiert und die alte Funktion umgeleitet, was so aussah:

<?php
  class HelperClass
  {
    public static function trimArray( $array )
    {
      return ArrayHelperClass::trim( $array );
    }
  }

  class ArrayHelperClass
  {
    public static function trim( $array )
    {
      // lösche alle leeren Elemente aus dem Array
      return $array;
    }
  }
?>

Ich glaube nicht, dass jemand dieses Refactoring anders angegangen hätte. Über den Sinn und Unsinn von Helperklassen können wir ein anderes mal diskutieren. Bitte schaut euch meinen Umbau genau an, wer von das Problem auf Anhieb sieht, der bekommt von mir einen kompletten Eintrag darüber, wie toll er ist. Ich muss aber noch dazu sagen, dass wir diese Funktion bereits mit Unittests abgedeckt hatten, uns aber dieser eine Spezialfall nicht eingefallen ist.

Nachdem diese umgeschriebene Funktion auf unseren Produktiv-Server gewechselt ist ging erstmal gar nichts mehr. Irgendwann konnten wir dann das Problem auf diese Methode eingrenzen. Ich könnte noch lange um den heißen Brei herum reden, aber ich glaub ich fange jetzt erstmal mit dem Problem an, bevor ich alle zu tode langweile.

Meine Kollegen und ich haben einfach den in PHP erlaubten call-by-reference Fall übersehen. Ruft man diese Funktion mit einem Verweis auf, z.B. mit

HelperClass::trimArray( &$array );

so funktioniert das ganze „natürlich“ in der ersten, alten Variante. PHP denkt aber gar nicht dran, diesen Verweis auch als Verweis an die trim Methode der neuen Klassen weiterreichen. Dass heißt, das $array jetzt halt nicht mehr nach überschrieben wird und kein neuer Wert übergeben wird. Ok ich gebe ja zu, dass man dem Entwickler ,der dort eine Referenz rein gegeben hat, ein paar runterhauen sollte, aber über 4 Jahre alten Legacy Code sollte man sich nie zu sehr aufregen. Als wir den Fehler dann endlich gefunden hatten, dauerte es auch nicht mehr wirklich lange, bis wir eine Lösung hatten. Prinzipiell ist es ganz einfach, man muss nur eine „unnötige“ Variable definieren.

<?php
  class HelperClass
  {
    public static function trimArray( $array )
    {
      $array = ArrayHelperClass::trim( $array );
      return $array;
    }
  }
>

Und schon klappt das ganze auch per Referenz. Da ich aber gerne unnötige Variablen vermeide, werden wir wohl am Montag erstmal eine neue Regel definieren, dass man eine solche Funktion nur so verwenden sollte, wie sie vorgesehen ist und zwar nicht für call-by-reference.

2 Comments