Dlaczego ważne jest by nie używać ponownie tych samych zmiennych lokalnych

W tym poście chciałbym pokazać do czego prowadzi pewna "dziadowska oszczędność" czyli mikrooptymalizacja w kodzie.

W ostatnim poście, opisałem niektóre zalecane (o zgrozo) mikrooptymalizacje które mają uczynić twój kod nieczytelnym i przy okazji zwiększyć jego wydajność (autor zapomniał dopisać że będzie to rzędu 3-4%).

Właśnie przed chwilą szukałem dość poważnego błędu w dość starym kawałku mojego kodu (połowa węzłów drzewa miała wartość null, zamiast liczby nieparzystej).

    /**
     * Metoda rekurencyjnie zaglebia sie w drzewo i tworzy strukture drzewiastą o zadanym stopniu głębokosci, wywolujemy ja
     * dla kazdego węzła poddrzewa
     *
     * @param int      $level            poziom głębokosci drzewa, gdy dojdziemy do 1 to Metoda sie zatrzymuje
     * @param bool     $insertNullFields Czy Metoda na tym poziomie ma wstawic puste miejsce na nowego przegranego czy nie
     * @param TreeNode $node             galaz dla ktorej bedziemy przeprowadzac operacje
     */
    protected function createChildNode($maxLevel, $insertNullFields, TreeNode $node, $currentLevel,
                                       $numeratorTableCounter = null, $roundID = null)
    {
        if ($roundID < 10 || $roundID == null) {
            echo "błąd";
        }

        if ($currentLevel == $maxLevel) {
            /* Dla obydwu dzieci tego węzła ktore beda ostatnimi węzłami w tym drzewie */
            for ($counter = 0; $counter < 2; $counter++) {
                /* Przypisz dla obecnie edytowanego węzła nowy węzeł jako dziecko, ID weź z numeratora poniewaz są
        * to puste węzły z miejscem na przegranych odpadajacych z turnieju, algorytm sie zatrzymuje,
        * dla dzieci dalej nie robimy juz nic */
                $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                /* Przypisz ten wezeł jako pojemnik na przegranych na odpowiednim poziomie i pozycji */
                $this->numeratorTable[$numeratorTableCounter][] = $newNode->ID;
            }
        } else {
            if ($currentLevel < $maxLevel) {
                /* Wstawiamy jedno puste pole i jedno sie dalej rozgałęzia, uruchomiono Metode z atrybutem True */
                if ($insertNullFields) {
                    /* Puste pole */
                    $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                    $node->children[0] = $newNode;
                    /* Wpisujemy do tabelki z polami przegranych */
                    $this->numeratorTable[$numeratorTableCounter][] = $newNode->ID;
                    /* Pole rozgaleziajace sie dalej (miejsce na bitwe i na jej wynik) */
                    $newNode = TreeNode::create($this->nodes, $node);
                    /* I dla drugiego dziecka wywolujemy rekurencyjnie tą Metode, tym razem z atrybutem "false", poziom zmniejszamy */
                    $this->createChildNode($maxLevel, false, $newNode, ($currentLevel + 1),
                        ($numeratorTableCounter + 1), ($roundID - 1));
                } else {
                    /* Metoda uruchomiona z atrybutem false (wstaw 2 galezie i im dzieciom tez uruchom ta Metode) */
                    for ($counter = 0; $counter < 2; $counter++) {
                        $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                        /* uruchamiamy tą Metode dla nowo wstawionego dziecka, z atrybutem true (wstaw jedno pole pełne i jedno puste) */
                        $this->createChildNode($maxLevel, true, $newNode, ($currentLevel + 1),
                            ($numeratorTableCounter), ($roundID - 1));
                    }
                }
            }
        }
    }

ok, to jest kod całej metody, nie będziemy się zagłębiać w to czy jest dobrze napisana, bo wiedza zgodnie z prawem moora rośnie wykładniczo z czasem, co daje taki efekt, że człowiek nie chce patrzeć na kod który pisał rok temu bo uważa go za shit.

oto kawałek który był błędny

//[...]
 if ($currentLevel < $maxLevel) {
                /* Wstawiamy jedno puste pole i jedno sie dalej rozgałęzia, uruchomiono Metode z atrybutem True */
                if ($insertNullFields) {
                    /* Puste pole */
                    $newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);
                    $node->children[0] = $newNode;
                    /* Wpisujemy do tabelki z polami przegranych */
                    $this->numeratorTable[$numeratorTableCounter][] = $newNode->ID;
                    /* Pole rozgaleziajace sie dalej (miejsce na bitwe i na jej wynik) */
// TUTAJ JEST BŁĄD - powinno być w parametrze jeszcze null,null, $roundID
                    $newNode = TreeNode::create($this->nodes, $node);
                    /* I dla drugiego dziecka wywolujemy rekurencyjnie tą Metode, tym razem z atrybutem "false", poziom zmniejszamy */
                    $this->createChildNode($maxLevel, false, $newNode, ($currentLevel + 1),
                        ($numeratorTableCounter + 1), ($roundID - 1));
                }
//[...]

jak widać mamy tutaj zmienną newNode użytą w dwóch miejscach. pierwsze użycie to

$newNode = TreeNode::create($this->nodes, $node, null, null, null, $roundID);

natomiast drugie to

$newNode = TreeNode::create($this->nodes, $node); // notabene ta druga również powinna mieć jeszcze 3 parametry: null,null, $roundID

W pierwszym momencie, pomyślałem sobie, (to dośc logiczny wniosek) że, po prostu omyłkowo mi się skopiował kod z pierwszego wywołania w miejsce drugiego, tylko jeszcze do tego wcieło mi 3 parametry i obiekt dobry zostal zastąpiony złym. W końcu jest to zrobione do jednej zmiennej $newNode.

Dopiero wnikliwsza analiza pozwoliła mi stwierdzić, że tak naprawde to powinny być utworzone dwie TreeNode, jedna na góre i jedna na dół. Pierwsza jest tworzona w $treeNode, zostaje dodana do drzewa, następnie druga TreeNode zostaje przypisana do $treeNode i również dodana do drzewa. Jednak te obiekty powinny się one znajdować w dwóch zmiennych - $upNewNode i $downNewNode, wtedy po spojrzeniu na metodę od razu bym wiedział że są to dwa rozdzielne obiekty, a nie jeden ciągły.

Dlatego nie powinno się "reużywać" starych lokalnych zmiennych dla nowych obiektów - jeżeli chcemy pisać kod poprawny semantycznie i taki który da się rozszyfrować nawet jeśli nie ma komentarzy - a komentarze z kodu chcemy w miare mozliwości usuwać i zastępować je czytelnym kodem - podwójnie użyta zmienna nie pozwala na odczytanie jej sensu z samego kodu. Nawet jeśli pierwszy obiekt jest uzywany w pierwszej części metody, a drugi w drugim. Panowie i panie! Robimy dwie zmienne, nie oszczędzamy na 2 bajtach pamięci bo później są z tego tytułu same problemy.