Czy lepiej jest strzec wywołania metody czy samej metody?

Piszę aplikację i doszedłem do tego:

private void SomeMethod()
{
    if (Settings.GiveApples)
    {
        GiveApples();
    }

    if (Settings.GiveBananas)
    {
        GiveBananas();
    }
}

private void GiveApples()
{
    ...
}

private void GiveBananas()
{
    ...
}

Wygląda to całkiem prosto. Istnieją pewne warunki i jeśli są prawdziwe, wywoływane są metody. Zastanawiałem się jednak, czy lepiej zrobić to w ten sposób:

private void SomeMethod()
{
    GiveApples();
    GiveBananas();
}

private void GiveApples()
{
    if (!Settings.GiveApples)
    {
        return;
    }

    ...
}

private void GiveBananas()
{
    if (!Settings.GiveBananas)
    {
        return;
    }

    ...
}

W drugim przypadku, każda z metod sama strażników, więc nawet jeśli którykolwiek z tych metod GiveAppleslub GiveBananasjest wywoływana z zewnątrz SomeMethod, mają zamiar być wykonywane tylko wtedy, gdy mają one prawidłową flagę w Ustawieniach.

Czy to coś, co powinienem uznać za problem?

W moim obecnym kontekście jest bardzo mało prawdopodobne, że te dwie metody zostaną wywołane spoza tej metody, ale nikt nigdy nie może tego zagwarantować.

Myślę, że strażnicy są czymś, czego metoda musi przestrzegać. W twoim przykładzie metoda nie może dać jabłek, jeśli Settings.GiveApples ma wartość false.

W takim przypadku strażnik zdecydowanie należy do metody. Zapobiega to przypadkowemu wywołaniu go z innego punktu w aplikacji bez uprzedniego sprawdzenia strażników.

Z drugiej strony, jeśli ustawienia dotyczą tylko metody wywoływania, a inna metoda w innym miejscu w kodzie może GiveApples niezależnie od ustawienia, to nie jest to strażnik i prawdopodobnie powinien znajdować się w kodzie wywołującym.

Umieść strażnika w obrębie samej metody. Konsument GiveApples()lub GiveBananas()nie powinien być odpowiedzialny za zarządzanie strażnikami GiveApples().

Z punktu widzenia projektu SomeMethod()powinien wiedzieć tylko, że potrzebuje owoców i nie powinien dbać o to, co aplikacja musi zrobić, aby to uzyskać. Abstrakcja pobierania owoców staje się nieszczelna, jeśli SomeMethod()jest odpowiedzialna za to, że istnieje globalne ustawienie, które umożliwia pobieranie niektórych owoców. Kaskada ta pojawia się, jeśli Twój mechanizm ochronny kiedykolwiek się zmienia, podobnie jak teraz wszystkie metody, które wymagają GetApples()lub GetBananas()muszą zostać zrefaktoryzowane osobno, aby wdrożyć tę nową ochronę. Bardzo łatwo jest zapomnieć o próbach uzyskania owoców bez tego sprawdzania podczas pisania kodu.

W tym scenariuszu należy wziąć pod uwagę sposób, w jaki aplikacja powinna zareagować, gdy Ustawienia nie pozwalają aplikacji na wydawanie owoców.

Zasadniczo często dobrym pomysłem jest rozdzielenie obowiązków testowania czegoś takiego jak ustawienia dostarczone zewnętrznie i „kodu podstawowego biznesu” GiveApples. Z drugiej strony dobrym pomysłem jest posiadanie funkcji grupujących razem to, co należy. Możesz osiągnąć oba cele, refaktoryzując kod w następujący sposób:

private void SomeMethod()
{
    GiveApplesIfActivated();
    GiveBananasIfActivated();
}

private void GiveApplesIfActivated()
{
    if (Settings.GiveApples)
    {
        GiveApples();
    }
}

private void GiveBananasIfActivated()
{
    if (Settings.GiveBananas)
    {
        GiveBananas();
    }
}

private void GiveApples()
{
    ...
}

private void GiveBananas()
{
    ...
}

Daje to większą szansę na przefakturowanie kodu GiveApplesi / lub GiveBananasw oddzielne miejsce bez żadnych zależności od Settingsklasy. Jest to oczywiście korzystne, gdy chcesz wywołać te metody w teście jednostkowym, który nie ma znaczenia Settings.

Jeśli jednak w twoim programie zawsze jest źle, pod żadnym pozorem, nawet w kontekście testowania, wywoływanie czegoś takiego jak GiveApplespoza kontekstem, w którym Settings.GiveApplesjest najpierw sprawdzane, a masz wrażenie, że podanie funkcji takiej jak GiveApplesbez Settingssprawdzania jest podatne na błędy , a następnie trzymaj się wariantu, w którym Settings.GiveApplestestujesz GiveApples.