Czy powinienem przekazać obiekt do konstruktora, czy utworzyć instancję w klasie?

10

Rozważ te dwa przykłady:

Przekazywanie obiektu do konstruktora

class ExampleA
{
  private $config;
  public function __construct($config)
  {
     $this->config = $config;
  }
}
$config = new Config;
$exampleA = new ExampleA($config);

Tworzenie instancji klasy

class ExampleB
{
  private $config;
  public function __construct()
  {
     $this->config = new Config;
  }
}
$exampleA = new ExampleA();

Jaki jest prawidłowy sposób obsługi dodawania obiektu jako właściwości? Kiedy powinienem używać jednego na drugim? Czy testy jednostkowe wpływają na to, czego powinienem użyć?

Więzień
źródło
Czy może być lepiej na codereview.stackexchange.com/questions ?
FrustratedWithFormsDesigner
9
@FrustratedWithFormsDesigner - nie nadaje się do recenzji kodu.
ChrisF

Odpowiedzi:

14

Myślę, że pierwszy da ci możliwość stworzenia configobiektu w innym miejscu i przekazania go ExampleA. Jeśli potrzebujesz wstrzyknięcia zależności, może to być dobre, ponieważ możesz zapewnić, że wszystkie intencje współużytkują ten sam obiekt.

Z drugiej strony, być może twój ExampleA wymaga nowego i czystego configobiektu, więc mogą wystąpić przypadki, w których drugi przykład jest bardziej odpowiedni, na przykład przypadki, w których każda instancja może potencjalnie mieć inną konfigurację.

FrustratedWithFormsDesigner
źródło
1
Zatem A nadaje się do testowania jednostkowego, B nadaje się do innych okoliczności ... dlaczego nie mieć obu? Często mam dwa konstruktory, jeden do ręcznego DI, drugi do normalnego użytkowania (używam Unity do DI, który wygeneruje konstruktory spełniające jego wymagania DI).
Ed James
3
@EdWoodcock tak naprawdę nie chodzi o testy jednostkowe w porównaniu do innych okoliczności. Obiekt albo wymaga zależności z zewnątrz, albo zarządza nim od wewnątrz. Nigdy jedno i drugie.
MattDavey,
9

Nie zapomnij o testowalności!

Zwykle jeśli zachowanie Exampleklasy zależy od konfiguracji, którą chcesz móc przetestować bez zapisywania / modyfikowania wewnętrznego stanu instancji klasy (tzn. Chcesz mieć proste testy pod kątem szczęśliwej ścieżki i niewłaściwej konfiguracji bez modyfikowania właściwości / członek Exampleklasy).

Dlatego wybrałbym pierwszą opcję ExampleA

Paweł
źródło
Dlatego wspomniałem o testowaniu - dziękuję, że to dodałem :)
Więzień
5

Jeśli obiekt jest odpowiedzialny za zarządzanie czasem życia zależności, wówczas można utworzyć obiekt w konstruktorze (i zrzucić go w destruktorze). *

Jeśli obiekt nie jest odpowiedzialny za zarządzanie czasem życia zależności, powinien zostać przekazany do konstruktora i zarządzany z zewnątrz (np. Przez kontener IoC).

W tym przypadku nie sądzę, że twoja klasa A powinna wziąć odpowiedzialność za utworzenie $ config, chyba że jest również odpowiedzialna za usunięcie go lub jeśli konfiguracja jest unikalna dla każdej instancji klasy A.

* Aby pomóc w testowaniu, konstruktor może odwoływać się do klasy / metody fabryki w celu skonstruowania zależności w swoim konstruktorze, zwiększając spójność i testowalność.

//Object which manages the lifetime of its dependency (C#):
public class ClassA : IDisposable
{
    public Config Config { get; private set; }

    public ClassA()
    {
        this.Config = new Config(); // Tightly coupled to Config class...
    }

    public void Dispose()
    {
        this.Config.Dispose();
    }
}

// Object which does not manage its dependency:
public class ClassA
{
    public Config Config { get; set; }

    public ClassA(Config config) // dependency may be injected...
    {
        this.Config = config;
    }
}

// Object which manages its dependency in a testable way:
public class ClassA : IDisposable
{
    public Config Config { get; private set; }

    public ClassA(IConfigFactory configFactory) // dependency may be mocked...
    {
        this.Config = configFactory.BuildConfig();
    }

    public void Dispose()
    {
        this.Config.Dispose();
    }
}
MattDavey
źródło
2

Niedawno miałem dokładnie tę samą dyskusję z naszym zespołem architektury i istnieją pewne subtelne powody, aby to zrobić w taki czy inny sposób. Sprowadza się to głównie do Dependency Injection (jak zauważyli inni) i tego, czy naprawdę masz kontrolę nad tworzeniem nowego obiektu w konstruktorze.

W twoim przykładzie, co jeśli twoja klasa Config:

a) ma nietrywialny przydział, taki jak metoda puli lub metoda fabryczna.

b) może nie zostać przydzielony. Przekazywanie go do konstruktora starannie pozwala uniknąć tego problemu.

c) jest w rzeczywistości podklasą Config.

Przekazywanie obiektu do konstruktora daje największą elastyczność.

JBRWilkinson
źródło
1

Poniższa odpowiedź jest błędna, ale zatrzymam ją, aby inni mogli się z niej uczyć (patrz poniżej)

W ExampleAmożna użyć tego samego Configwystąpienia w wielu klasach. Jeśli jednak Configw całej aplikacji powinna znajdować się tylko jedna instancja, rozważ zastosowanie wzorca Singleton, Configaby uniknąć wielu instancji Config. A jeśli Configjest singletonem, możesz zamiast tego wykonać następujące czynności:

class ExampleA
{
  private $config;
  public function __construct()
  {
     $this->config = Config->getInstance();
  }
}
$exampleA = new ExampleA();

Z ExampleBdrugiej strony, zawsze otrzymasz osobne wystąpienie Configdla każdego wystąpienia ExampleB.

Która wersja powinna zostać zastosowana, zależy od tego, jak aplikacja będzie obsługiwać wystąpienia Config:

  • jeśli każda instancja ExampleXpowinna mieć osobną instancję Config, idź z ExampleB;
  • jeśli każde wystąpienie ExampleXbędzie dzielić jedno (i tylko jedno) wystąpienie Config, użyj ExampleA with Config Singleton;
  • jeśli instancje ExampleXmogą używać różnych instancji Config, trzymaj się ExampleA.

Dlaczego przejście Configna Singleton jest złe:

Muszę przyznać, że wczoraj dowiedziałem się o wzorze Singleton (czytając książkę wzorców projektowych Head First ). Naiwnie poszedłem i zastosowałem go w tym przykładzie, ale jak wielu zauważyło, jeden sposób jest inny (niektórzy byli bardziej tajemniczy i mówili tylko „robisz to źle!”), To nie jest dobry pomysł. Tak więc, aby inni nie popełnili tego samego błędu, który właśnie popełniłem, poniżej znajduje się podsumowanie, dlaczego wzorzec Singleton może być szkodliwy (na podstawie komentarzy i tego, co go znalazłem w Google):

  1. Jeśli ExampleApobierze własne odwołanie do Configinstancji, klasy będą ściśle powiązane. Nie będzie ExampleAmożliwości użycia innej wersji Config(powiedzmy podklasy). Jest to okropne, jeśli chcesz przetestować ExampleAprzy użyciu instancji makiety, Configponieważ nie ma sposobu, aby to zrobić ExampleA.

  2. Założeniem będzie jeden i tylko jeden przypadek, który Configmoże się teraz utrzymywać , ale nie zawsze możesz być pewien, że to samo będzie obowiązywać w przyszłości . Jeśli w pewnym momencie okaże się, że Configpożądanych będzie wiele instancji , nie ma możliwości osiągnięcia tego bez przepisania kodu.

  3. Chociaż jedno i jedyne wystąpienie Configmoże być prawdą przez całą wieczność, może się zdarzyć, że będziesz chciał móc skorzystać z jakiejś podklasy Config(wciąż mając tylko jedno wystąpienie). Ale, ponieważ kod pobiera bezpośrednio poprzez wystąpienie getInstance()o Config, który jest staticmetoda, nie ma sposobu, aby uzyskać podklasy. Ponownie kod musi zostać przepisany.

  4. Fakt, że ExampleAzastosowania Configzostaną ukryte, przynajmniej podczas przeglądania interfejsu API ExampleA. Może to być, ale nie musi, zła rzecz, ale osobiście uważam, że jest to niekorzystne; na przykład, utrzymując, nie ma prostego sposobu, aby dowiedzieć się, na które klasy będą miały wpływ zmiany, Configbez patrzenia na implementację każdej innej klasy.

  5. Nawet jeśli fakt, że ExampleAużywa się Singletona Config , sam w sobie nie stanowi problemu, może on stać się problemem z testowego punktu widzenia. Obiekty Singleton będą nosić stan, który będzie trwał do momentu zakończenia aplikacji. Może to stanowić problem podczas uruchamiania testów jednostkowych, ponieważ chcesz, aby jeden test był izolowany od drugiego (tj. Że wykonanie jednego testu nie powinno wpływać na wynik innego). Aby to naprawić, obiekt Singleton musi zostać zniszczony między każdym uruchomieniem testowym (potencjalnie konieczne ponowne uruchomienie całej aplikacji), co może być czasochłonne (nie wspominając o żmudnych i irytujących).

Powiedziawszy to, cieszę się, że popełniłem ten błąd tutaj, a nie podczas wdrażania prawdziwej aplikacji. W rzeczywistości zastanawiałem się nad przepisaniem mojego najnowszego kodu w celu użycia wzorca Singleton dla niektórych klas. Chociaż mógłbym z łatwością cofnąć zmiany (oczywiście wszystko jest przechowywane w SVN), nadal zmarnowałbym czas na robienie tego.

gablin
źródło
4
Nie polecam tego robić ... w ten sposób ściśle łączysz klasę ExampleAi Config- co nie jest dobrą rzeczą.
Paul
@Paul: To prawda. Dobry haczyk, nie myślałem o tym.
gablin
3
Zawsze odradzam używanie Singletonów ze względu na testowalność. Są to zasadniczo zmienne globalne i nie można kpić z zależności.
MattDavey,
4
Zawsze zalecałbym ponowne użycie Singletonów ze względu na to, że zrobiłeś to źle.
Raynos,
1

Najprostszą rzeczą do zrobienia jest kilka ExampleAdo Config. Powinieneś zrobić najprostszą rzecz, chyba że istnieje ważny powód, aby zrobić coś bardziej złożonego.

Jednym z powodów, dla oddzielenia ExampleAi Configbyłoby poprawić testowalności ExampleA. Bezpośrednie sprzężenie się pogorszyć testowalności ExampleAjeśli Configma metod jest powolna, kompleks, lub szybko się zmienia. Do testowania metoda jest powolna, jeśli działa dłużej niż kilka mikrosekund. Jeśli wszystkie metody Configsą proste i szybkie, wtedy biorę proste podejście i jest bezpośrednio parę ExampleAdo Config.

Kevin Cline
źródło
1

Twój pierwszy przykład to przykład wzoru wstrzykiwania zależności. Klasa z zewnętrzną zależnością jest przekazywana przez konstruktora, settera itp.

Takie podejście powoduje luźny kod. Większość ludzi uważa, że ​​luźne sprzężenie jest dobrą rzeczą, ponieważ można łatwo zastąpić konfigurację w przypadkach, w których jedna konkretna instancja obiektu musi być skonfigurowana inaczej niż inne, można przekazać próbny obiekt konfiguracyjny do testowania, i tak na.

Drugie podejście jest bliższe wzorcowi twórcy GRASP. W takim przypadku obiekt tworzy własne zależności. Powoduje to ściśle powiązany kod, co może ograniczyć elastyczność klasy i utrudnić testowanie. Jeśli potrzebujesz jednej instancji klasy, aby mieć inną zależność od innych, jedyną opcją jest jej podklasę.

Może to być jednak odpowiedni wzorzec w przypadkach, w których długość życia zależnego obiektu jest podyktowana czasem życia zależnego obiektu, i gdy zależny obiekt nie jest używany nigdzie poza obiektem zależnym od niego. Normalnie radziłbym DI jako pozycję domyślną, ale nie musisz całkowicie wykluczać drugiego podejścia, o ile masz świadomość jego konsekwencji.

GordonM
źródło
0

Jeśli twoja klasa nie udostępnia $configklas zewnętrznych, utworzę ją w konstruktorze. W ten sposób zachowujesz prywatność swojego stanu wewnętrznego.

Jeśli $configwymaga to poprawnego ustawienia własnego stanu wewnętrznego (np. Wymaga połączenia z bazą danych lub zainicjowania niektórych pól wewnętrznych) przed użyciem, wówczas sensowne jest odroczenie inicjalizacji do jakiegoś zewnętrznego kodu (być może klasy fabrycznej) i wstrzyknięcie go konstruktor. Lub, jak zauważyli inni, jeśli trzeba go udostępnić innym obiektom.

TMN
źródło
0

Przykład A jest oddzielony od konkretnej klasy Config, co jest dobre , pod warunkiem otrzymania obiektu, jeśli nie jest to typ Config, ale typ abstrakcyjnej nadklasy Config.

Przykład B jest silnie sprzężony z konkretną konfiguracją klasy, która jest zła .

Tworzenie wystąpienia obiektu tworzy silne połączenie między klasami. Należy to zrobić w klasie Factory.

Tulains Córdova
źródło