Czy używanie „nowego” w konstruktorze jest zawsze złe?

37

Czytałem, że użycie „nowego” w konstruktorze (dla dowolnych obiektów innych niż te o prostej wartości) jest złą praktyką, ponieważ uniemożliwia testowanie jednostkowe (ponieważ wtedy też trzeba stworzyć tych współpracowników i nie można ich wyśmiewać). Ponieważ tak naprawdę nie mam doświadczenia w testowaniu jednostkowym, staram się zebrać kilka zasad, których najpierw się nauczę. Czy ta zasada jest ogólnie ważna, niezależnie od używanego języka?

Ezoela Vacca
źródło
9
Nie uniemożliwia testowania. Utrudnia to jednak utrzymanie i testowanie kodu. Przeczytaj na przykład nowy klej .
David Arno
38
„zawsze” jest zawsze niepoprawne. :) Istnieją najlepsze praktyki, ale istnieje wiele wyjątków.
Paul
63
Jaki to język? newoznacza różne rzeczy w różnych językach.
user2357112 obsługuje Monikę
13
To pytanie zależy trochę od języka, prawda? Nie wszystkie języki mają nawet newsłowa kluczowe. O jakie języki pytasz?
Bryan Oakley,
7
Głupia zasada. Nieużywanie zastrzyku zależności, gdy powinieneś mieć bardzo mało wspólnego ze słowem kluczowym „new”. Zamiast powiedzieć, że „używanie nowego jest problemem, jeśli używasz go do zerwania inwersji zależności”, po prostu powiedz „nie niszcz inwersji zależności”.
Matt Timmermans

Odpowiedzi:

36

Zawsze są wyjątki, a ja nie zgadzam się z „zawsze” w tytule, ale tak, ta wytyczna jest ogólnie ważna i ma zastosowanie również poza konstruktorem.

Użycie nowego w konstruktorze narusza D w SOLID (zasada inwersji zależności). Utrudnia to testowanie kodu, ponieważ testowanie jednostkowe polega na izolacji; trudno jest wyodrębnić klasę, jeśli ma konkretne odniesienia.

Nie chodzi jednak tylko o testy jednostkowe. Co jeśli chcę skierować repozytorium na dwie różne bazy danych jednocześnie? Możliwość przejścia we własnym kontekście pozwala mi tworzyć instancje dwóch różnych repozytoriów wskazujących różne lokalizacje.

Nieużywanie nowego w konstruktorze powoduje, że kod jest bardziej elastyczny. Dotyczy to również języków, które mogą wykorzystywać konstrukcje inne niż newdo inicjowania obiektów.

Jednak najwyraźniej musisz zachować rozsądek. Jest wiele razy, kiedy dobrze jest używać newlub gdzie lepiej byłoby tego nie robić, ale nie będziesz miał negatywnych konsekwencji. W pewnym momencie newtrzeba go nazwać. Uważaj tylko na wywołanie newwewnątrz klasy, od której zależy wiele innych klas.

Robienie czegoś takiego jak inicjowanie pustej kolekcji prywatnej w konstruktorze jest w porządku, a wstrzykiwanie jej byłoby absurdalne.

Im więcej odwołań do niej ma klasa, tym ostrożniej nie powinieneś dzwonić newz jej wnętrza.

TheCatWhisperer
źródło
12
Naprawdę nie widzę żadnej wartości w tej regule. Istnieją zasady i wytyczne dotyczące unikania ścisłego łączenia w kodzie. Ta reguła wydaje się dotyczyć dokładnie tego samego problemu, z tym wyjątkiem, że jest niesamowicie zbyt restrykcyjna, nie dając nam żadnej dodatkowej wartości. Więc o co chodzi? Dlaczego tworzenie fałszywego obiektu nagłówka dla mojej implementacji LinkedList zamiast zajmować się tymi wszystkimi specjalnymi przypadkami, które wynikają z head = nulljakiejkolwiek poprawy kodu? Dlaczego pozostawienie dołączonych kolekcji jako zerowych i tworzenie ich na żądanie byłoby lepsze niż robienie tego w konstruktorze?
Voo
22
Nie rozumiesz sedna sprawy. Tak, moduł trwałości jest absolutnie czymś, na czym moduł wyższego poziomu nie powinien polegać. Ale „nigdy nie używaj nowego” nie wynika z „niektóre rzeczy należy wstrzykiwać, a nie łączyć bezpośrednio”. Jeśli weźmiesz swoją zaufaną kopię Agile Software Development, zobaczysz, że Martin ogólnie mówi o modułach, a nie o pojedynczych klasach: „Moduły wysokiego poziomu zajmują się politykami wysokiego poziomu aplikacji. Zasady te na ogół nie dbają o szczegóły, które implementują im."
Voo
11
Chodzi o to, aby unikać zależności między modułami - szczególnie między modułami o różnych poziomach abstrakcji. Ale moduł może być więcej niż jedną klasą i po prostu nie ma sensu stawiać interfejsów między ściśle połączonym kodem tej samej warstwy abstrakcji. W dobrze zaprojektowanej aplikacji zgodnej z zasadami SOLID nie każda klasa powinna implementować interfejs i nie każdy konstruktor powinien zawsze przyjmować interfejsy jako parametry.
Voo
18
Głosowałem za tym, ponieważ to dużo modnych zup i mało dyskusji na temat praktycznych względów. Jest coś w repozytorium dwóch baz danych, ale szczerze mówiąc, to nie jest przykład z prawdziwego świata.
jpmc26
5
@ jpmc26, BJoBnh Nie wchodząc w to, czy akceptuję tę odpowiedź, czy nie, punkt jest wyrażony bardzo wyraźnie. Jeśli uważasz, że „zasada inwersji zależności”, „konkretne odniesienie” lub „inicjalizacja obiektu” to tylko modne słowa, to tak naprawdę nie wiesz, co one oznaczają. To nie jest wina odpowiedzi.
R. Schmitz
50

Chociaż opowiadam się za użyciem konstruktora do zainicjowania nowej instancji zamiast tworzenia wielu innych obiektów, obiekty pomocnicze są w porządku i musisz ocenić, czy coś jest wewnętrznym pomocnikiem, czy nie.

Jeśli klasa reprezentuje kolekcję, może mieć wewnętrzną tablicę pomocniczą, listę lub zestaw skrótów. Użyłby newdo stworzenia tych pomocników i byłoby to uważane za całkiem normalne. Klasa nie oferuje zastrzyku w celu korzystania z różnych wewnętrznych pomocników i nie ma powodu. W takim przypadku chcesz przetestować publiczne metody obiektu, które mogą przejść do gromadzenia, usuwania i zastępowania elementów w kolekcji.


W pewnym sensie klasowa konstrukcja języka programowania jest mechanizmem do tworzenia abstrakcji wyższego poziomu, a my tworzymy takie abstrakcje, aby wypełnić lukę między domeną problemową a prymitywami języka programowania. Jednak mechanizm klasowy jest tylko narzędziem; różni się w zależności od języka programowania, a niektóre abstrakty domenowe w niektórych językach wymagają po prostu wielu obiektów na poziomie języka programowania.

Podsumowując, musisz się zastanowić, czy abstrakcja wymaga tylko jednego lub więcej obiektów wewnętrznych / pomocniczych, podczas gdy osoba wywołująca nadal jest postrzegana jako pojedyncza abstrakcja, czy też inne obiekty byłyby lepiej narażone na kontakt z dzwoniącym, aby utworzyć kontrola zależności, co byłoby sugerowane, na przykład, gdy wywołujący widzi te inne obiekty podczas korzystania z klasy.

Erik Eidt
źródło
4
+1. Dokładnie tak jest. Musisz określić, jakie jest zamierzone zachowanie klasy, a to określa, które szczegóły implementacji należy ujawnić, a które nie. Jest też rodzaj subtelnej gry intuicyjnej, w którą musisz grać, określając, jaka jest „odpowiedzialność” klasy.
jpmc26
27

Nie wszyscy współpracownicy są wystarczająco interesujący, aby osobno testować jednostki, możesz (pośrednio) przetestować ich za pośrednictwem klasy hostingowej / instancji. Może to nie zgadzać się z pomysłem niektórych osób, że trzeba przetestować każdą klasę, każdą metodę publiczną itp., Szczególnie podczas wykonywania testu po. Korzystając z TDD, możesz zreformować tego „współpracownika”, wyodrębniając klasę, w której jest ona już w pełni testowana od pierwszego testowego procesu.

Joppe
źródło
14
Not all collaborators are interesting enough to unit-test separatelykoniec historii :-), Ta sprawa jest możliwa i nikt nie odważył się wspomnieć. @Joppe Zachęcam do szczegółowego opracowania odpowiedzi. Na przykład możesz dodać kilka przykładów klas, które są jedynie szczegółami implementacji (nie nadającymi się do zastąpienia) i jak można je później wyodrębnić, jeśli uznamy to za konieczne.
Laiv
Modele domen @Laiv są zazwyczaj konkretne, nieabstrakcyjne, nie chodzi tu o wstrzykiwanie zagnieżdżonych obiektów. Obiekt o prostej wartości / obiekt złożony bez logiki również jest kandydatem.
Joppe
4
+1, absolutnie. Jeżeli język jest tak skonfigurowany, że ma zadzwonić new File()do niczego związanych plików, nie ma sensu zakazywanie to wezwanie. Co zamierzasz zrobić, napisz testy regresji w stosunku do Filemodułu stdlib ? Raczej nie. Z drugiej strony, wzywanie newdo wymyślonej przez siebie klasy jest bardziej wątpliwe.
Kilian Foth
7
@KilianFoth: Powodzenia testuje jednak wszystko, co bezpośrednio wywołuje new File().
Phoshi
1
To zgadywanie . Możemy znaleźć przypadki, w których nie ma to sensu, przypadki, w których nie jest to użyteczne oraz przypadki, w których ma to sens i jest przydatne. To kwestia potrzeb i preferencji.
Laiv
13

Ponieważ tak naprawdę nie mam doświadczenia w testowaniu jednostkowym, staram się zebrać kilka zasad, których najpierw się nauczę.

Uważnie ucz się „reguł” dotyczących problemów, których nigdy nie spotkałeś. Jeśli natkniesz się na jakieś „zasady” lub „najlepszych praktyk”, chciałbym zaproponować znalezienie prosty przykład zabawki, gdzie ta zasada jest „niby” mają być stosowane, i stara się rozwiązać ten problem samodzielnie , ignorując to, co mówi, że „zasada”.

W takim przypadku możesz spróbować wymyślić 2 lub 3 proste klasy i niektóre zachowania, które powinny zastosować. Zaimplementuj klasy w dowolny sposób i napisz test jednostkowy dla każdego zachowania. Zrób listę problemów, które napotkałeś, np. Jeśli zacząłeś od pracy w jedną stronę, a potem musiał wrócić i zmienić to później; jeśli masz wątpliwości co do tego, jak rzeczy mają do siebie pasować; jeśli denerwujesz się pisaniem bojlera; itp.

Następnie spróbuj rozwiązać ten sam problem, postępując zgodnie z „zasadą”. Ponownie zrób listę napotkanych problemów. Porównaj listy i zastanów się, które sytuacje mogą być lepsze, jeśli zastosujesz się do reguły, a które nie.


Jeśli chodzi o twoje aktualne pytanie, preferuję podejście oparte na portach i adapterach , w którym rozróżniamy „logikę podstawową” i „usługi” (jest to podobne do rozróżnienia między czystymi funkcjami a skutecznymi procedurami).

Podstawowa logika polega na obliczaniu rzeczy „wewnątrz” aplikacji w oparciu o domenę problemową. To może zawierać zajęcia takie jak User, Document, Order, Invoice, itd. Dobrze jest mieć podstawowe klasy zadzwonić newdo innych klas podstawowych, ponieważ są one „wewnętrzne” szczegóły wykonania. Na przykład utworzenie i Ordermoże również utworzyć Invoicei Documentwyszczególnić to, co zostało zamówione. Nie trzeba kpić z nich podczas testów, ponieważ to są rzeczy, które chcemy przetestować!

Porty i adaptery to sposób, w jaki podstawowa logika współdziała ze światem zewnętrznym. To gdzie takie rzeczy jak Database, ConfigFile, EmailSenderitp żyć. Są to rzeczy, które utrudniają testowanie, dlatego wskazane jest, aby utworzyć je poza podstawową logiką i przekazać je w razie potrzeby (z zastrzykiem zależności lub jako argumenty metody itp.).

W ten sposób podstawowa logika (która jest częścią specyficzną dla aplikacji, w której ważna logika biznesowa żyje i podlega największej rezygnacji) może zostać przetestowana samodzielnie, bez konieczności dbania o bazy danych, pliki, wiadomości e-mail itp. Możemy po prostu podać kilka przykładowych wartości i sprawdzić, czy otrzymujemy właściwe wartości wyjściowe.

Porty i adaptery można testować osobno, wykorzystując symulacje bazy danych, systemu plików itp. Bez konieczności dbania o logikę biznesową. Możemy po prostu podać niektóre przykładowe wartości i upewnić się, że są one przechowywane / czytane / wysyłane / itp. odpowiednio.

Warbo
źródło
6

Pozwólcie, że odpowiem na pytanie, zbierając to, co uważam za kluczowe punkty. Zacytuję niektórych użytkowników za zwięzłość.

Zawsze są wyjątki, ale tak, ta reguła jest ogólnie ważna i ma zastosowanie również poza konstruktorem.

Użycie nowego w konstruktorze narusza D w SOLID (zasada inwersji zależności). Utrudnia to testowanie kodu, ponieważ testowanie jednostkowe polega na izolacji; trudno jest wyodrębnić klasę, jeśli ma konkretne odniesienia.

-TheCatWhisperer-

Tak, stosowanie newwewnętrznych konstruktorów często prowadzi do wad projektowych (na przykład ścisłego połączenia), co czyni nasz projekt sztywnym. Trudne do przetestowania tak, ale nie niemożliwe. Właściwością w tym przypadku jest odporność (tolerancja na zmiany) 1 .

Niemniej powyższy cytat nie zawsze jest prawdziwy. W niektórych przypadkach może istnieć klasy , które są przeznaczone do ściśle sprzężone . David Arno skomentował kilka.

Istnieją oczywiście wyjątki, w których klasa jest niezmiennym obiektem wartości , szczegółem implementacji itp. Tam, gdzie powinny być ściśle powiązane .

-David Arno-

Dokładnie. Niektóre klasy (na przykład klasy wewnętrzne) mogą być jedynie szczegółami implementacji klasy głównej. Są one przeznaczone do testowania wraz z główną klasą i niekoniecznie muszą być wymienialne lub rozszerzane.

Co więcej, jeśli nasz SOLIDNY kult zmusi nas do wyodrębnienia tych klas , możemy naruszyć inną dobrą zasadę. Tak zwane Prawo Demetera . Z drugiej strony uważam, że jest to bardzo ważne z punktu widzenia projektowania.

Tak więc prawdopodobna odpowiedź, jak zwykle, zależy . Używanie newkonstruktorów wewnętrznych może być złą praktyką. Ale nie zawsze systematycznie.

Musimy więc ocenić, czy klasy są szczegółami implementacji (w większości przypadków nie będą) klasy głównej. Jeśli tak, zostaw je w spokoju. Jeśli nie są, rozważ techniki takie jak Składanie korzenia lub Wstrzykiwanie zależności przez IoC Containers .


1: głównym celem SOLID nie jest uczynienie naszego kodu bardziej testowalnym. Ma to na celu uczynienie naszego kodu bardziej odpornym na zmiany. Bardziej elastyczny, a zatem łatwiejszy do przetestowania

Uwaga: David Arno, TheWhisperCat, mam nadzieję, że nie masz nic przeciwko, że zacytowałem cię.

Laiv
źródło
3

Jako prosty przykład rozważ następujący pseudokod

class Foo {
  private:
     class Bar {}
     class Baz inherits Bar {}
     Bar myBar
  public:
     Foo(bool useBaz) { if (useBaz) myBar = new Baz else myBar = new Bar; }
}

Ponieważ newjest czysty detal wdrożenie Fooi oba Foo::Bari Foo::Bazsą częścią Foo, podczas testowania jednostka Foonie ma sensu w szyderczy części Foo. Kpisz z części na zewnątrz tylko Foo podczas testów jednostkowych Foo.

MSalters
źródło
-3

Tak, użycie „nowego” w klasach głównych aplikacji to zapach kodu. Oznacza to, że blokujesz klasę przy użyciu określonej implementacji i nie będziesz w stanie zastąpić innej. Zawsze wybieraj wstrzykiwanie zależności do konstruktora. W ten sposób nie tylko będziesz w stanie łatwo wstrzyknąć fałszywe zależności podczas testowania, ale sprawi, że twoja aplikacja będzie bardziej elastyczna, umożliwiając szybkie zastępowanie różnych implementacji w razie potrzeby.

EDYCJA: Dla downvoters - oto link do książki o rozwoju oprogramowania oznaczającej „nowy” jako możliwy zapach kodu: https://books.google.com/books?id=18SuDgAAQBAJ&lpg=PT169&dq=new%20keyword%20code%20smell&pg=PT169 # v = na stronie i q = nowy% 20 słowo kluczowe% 20code% 20smell & f = false

Eternal21
źródło
15
Yes, using 'new' in your non-root classes is a code smell. It means you are locking the class into using a specific implementation, and will not be able to substitute another.Dlaczego to jest problem? Nie każda zależność w drzewie zależności powinna być dostępna do zastąpienia
Laiv
4
@Paul: Posiadanie domyślnej implementacji oznacza, że ​​bierzesz ściśle powiązane odwołanie do konkretnej klasy określonej jako domyślna. Nie oznacza to jednak, że jest to tak zwany „zapach kodu”.
Robert Harvey
8
@EzoelaVacca: Byłbym ostrożny w używaniu słów „zapach kodu” w dowolnym kontekście. To tak jakby powiedzieć „republikanin” lub „demokrata”; co w ogóle oznaczają te słowa? Gdy tylko podasz coś takiego, przestajesz myśleć o prawdziwych problemach i przestaje się uczyć.
Robert Harvey
4
„Bardziej elastyczny” nie jest automatycznie „lepszy”.
whatsisname
4
@EzoelaVacca: Używanie newsłowa kluczowego nie jest złą praktyką i nigdy nie było. Liczy się sposób korzystania z narzędzia. Na przykład nie używasz młota, w którym wystarczy młot kulowy.
Robert Harvey