Code Smell: Temporary Field

Czasami obserwujemy obiekty, w których pewna zmienna instancji otrzymuje wartość tylko w niektórych okolicznościach. Taki kod trudno się czyta, gdyż z reguły spodziewamy się, że obiekt potrzebuje wszystkich swoich zmiennych. Próby zrozumienia, dlaczego obiekt zawiera zmienną, która nie jest używana, mogą doprowadzić do poważnej irytacji.

Martin Fowler, Kent Beck

W części artykułów z jakimi się spotkałam zapaszek ten zmienił wydźwięk z „obiekty, w których pewna zmienna instancji otrzymuje wartość tylko w niektórych okolicznościach.” na:

1. „A Temporary field is a variable that isn’t really needed, or used in a very limited situation” ~Javier Arroyo, 2020 [artykuł]
2. „If you have a class instance variable that has only been used once, you have a code smell called Temporary Fields.” ~www.codegrip.tech
3. „Temporary Fields code smell happens when you have a class instance variables that have been used only sometimes.” ~Mohamed Aladdin, 2018 [artykuł]

Ja się jednak skupię tutaj na definicji tego zapaszku w jego postaci z książki Fowlera.

Poniżej możesz zobaczyć bardzo okrojoną klasę SomeClass. Klasa ta ma metodę doSomething. W tej metodzie możesz zobaczyć, że najpierw coś robi, potem wpada w sprawdzenie warunku i jeśli jest prawdziwy to wychodzi z metody. W przypadku, gdy nie jest prawdziwy to ustawia wartość zmiennej $someValue. Następnie przechodzi do wykonania metody doSomethingTwo i w niej operuje na tej ustawionej zmiennej.

class SomeClass
{
    private string $someValue;

    public function doSomething(int $id): bool
    {
        // .. do some staff

        if ($this->checkSomething($id)) {
            return false;
        }
        
        $this->someValue = "TEXT";
        
        return $this->doSomethingTwo();
    }

    private function doSomethingTwo(): bool
    {
        if ($this->someValue === 'text') {
            // .. do something
            return true;
        }
        
        return false;
    }

    private function checkSomething(int $id): bool
    {
        // ...

        return true;
    }
}

Dlaczego to jest zapaszek?

Powiedzmy, że kolejna osoba spojrzy na tą klasę, chce dopisać kolejną metodę publiczną, która korzysta z metody doSomethingTwo. Może się mocno zaskoczyć, że zawsze dostanie z niej false. Po „krótkim” debugu zauważa, że zmienna someValue, nie zawsze ma przypisaną wartość.

Innym problematycznym przypadkiem będzie, gdy ktoś z jakiegoś powodu będzie chciał wynieść wykonanie metody doSomethingTwo na początek metody. Również może się zaskoczyć co tu się stało i dlaczego wykonanie się tak szybko kończy.

Jak można to rozwiązać?

Obecnie widzę 2 sposoby

1. Metoda doSomethingTwo przyjmuje w parametrze zmienną someValue. Po takiej zmianie myląca prywatna zmienna nie będzie już potrzebna.
2. Zmienną i zależne od niej metody wydzielamy do osobnej klasy.

Znasz jakieś inne rozwiązanie?

Podziel się w komentarzu swoimi przemyśleniami na ten temat 😉

A może w Twojej opinii to nie jest zapaszek? 🤔

Źródła

• książka: Refaktoryzacja. Ulepszanie struktury istniejącego kodu, Martin Fowler we współpracy z Kentem Beckiem
• www: Bad Smells in Software – a Taxonomy and an Empirical Study, Mika Mäntylä
• www: Temporary Field, Aleksander Shvets
• www: Code Smells, Jeff Atwood
• www: Temporary Field code smell, Mark Seemann