Uzasadniona „prawdziwa praca” w konstruktorze?

23

Pracuję nad projektem, ale wciąż uderzam w przeszkodę. Mam określoną klasę (ModelDef), która jest zasadniczo właścicielem złożonego drzewa węzłów zbudowanego przez analizowanie schematu XML (think DOM). Chcę przestrzegać dobrych zasad projektowania (SOLID) i upewnić się, że powstały system jest łatwo testowalny. Mam zamiar używać DI do przekazywania zależności do konstruktora ModelDef (aby w razie potrzeby można je było łatwo wymienić podczas testowania).

Mam jednak problem z tworzeniem drzewa węzłów. To drzewo będzie składało się wyłącznie z prostych obiektów „wartościowych”, które nie będą musiały być niezależnie testowane. (Jednak nadal mogę przekazać Fabrykę abstrakcyjną do ModelDef, aby pomóc w tworzeniu tych obiektów.)

Ale ciągle czytam, że konstruktor nie powinien wykonywać żadnej prawdziwej pracy (np. Flaw: Constructor robi prawdziwą pracę ). Ma to dla mnie idealny sens, jeśli „prawdziwa praca” oznacza konstruowanie obiektów zależnych od ciężkiej wagi, które później można chcieć usunąć do testów. (Należy je przekazać przez DI.)

Ale co z lekkimi obiektami wartościowymi, takimi jak to drzewo węzłów? Drzewo trzeba gdzieś stworzyć, prawda? Dlaczego nie przez konstruktora ModelDef (używając, powiedzmy, metody buildNodeTree ())?

Naprawdę nie chcę tworzyć drzewa węzłów poza ModelDef, a następnie przekazywać go (za pośrednictwem konstruktora DI), ponieważ tworzenie drzewa węzłów przez analizowanie schematu wymaga znacznej ilości złożonego kodu - kodu, który należy dokładnie przetestować . Nie chcę przenosić go do kodu „przyklejającego” (który powinien być względnie trywialny i prawdopodobnie nie będzie bezpośrednio testowany).

Myślałem o umieszczeniu kodu w celu utworzenia drzewa węzłów w oddzielnym obiekcie „konstruktora”, ale waham się nazwać go „konstruktorem”, ponieważ tak naprawdę nie pasuje on do wzorca konstruktora (który wydaje się być bardziej zainteresowany wyeliminowaniem teleskopu konstruktorów). Ale nawet jeśli nazwałbym to czymś innym (np. NodeTreeConstructor), nadal wydaje mi się, że to trochę hack, aby uniknąć konstruowania ModelDef przez drzewo węzłów. Musi być gdzieś zbudowany; dlaczego nie w obiekcie, który będzie jego właścicielem?

Gurtz
źródło
7
Jednak zawsze powinieneś uważać na takie ogólne stwierdzenia. Ogólną zasadą jest kodowanie w sposób przejrzysty, funkcjonalny, łatwy do przetestowania, ponownego użycia i konserwacji, bez względu na to, jaki to może być, w zależności od sytuacji. Jeśli nic nie zyskujesz poza złożonością kodu i nieporozumieniami, próbując postępować zgodnie z taką „zasadą”, to nie jest to odpowiednia reguła dla twojej sytuacji. Wszystkie te „wzorce” i funkcje językowe są narzędziami; użyj najlepszego dla swojej konkretnej pracy.
Jason C,

Odpowiedzi:

26

Poza tym, co zasugerował Ross Patterson, zastanów się nad tym stanowiskiem, które jest dokładnie odwrotne:

  1. Weź maksymy, takie jak „Nie wykonuj żadnej prawdziwej pracy w swoich konstruktorach” z dodatkiem ziarenka soli.

  2. Konstruktor jest tak naprawdę niczym innym jak metodą statyczną. Tak więc strukturalnie nie ma naprawdę dużej różnicy między:

    a) prosty konstruktor i kilka złożonych statycznych metod fabryki, oraz

    b) prosty konstruktor i kilka bardziej złożonych konstruktorów.

Znaczna część negatywnego sentymentu do wykonywania jakiejkolwiek rzeczywistej pracy w konstruktorach pochodzi z pewnego okresu w historii C ++, kiedy toczyła się debata na temat tego, w jakim stanie pozostanie obiekt, jeśli zostanie zgłoszony wyjątek w konstruktorze i czy w takim przypadku należy wywołać destruktor. Ta część historii C ++ się skończyła i problem został rozwiązany, podczas gdy w językach takich jak Java nigdy nie było takiego problemu na początek.

Moim zdaniem, jeśli po prostu unikniesz używania neww konstruktorze ((jak wskazuje zamiar zastosowania Dependency Injection)), powinieneś mieć się dobrze. Śmieję się ze stwierdzeń typu „logika warunkowa lub zapętlenie w konstruktorze jest znakiem ostrzegawczym błędu”.

Poza tym osobiście wyprowadziłbym logikę parsowania XML z konstruktora, nie dlatego, że posiadanie złożonej logiki w konstruktorze jest złe, ale dlatego, że dobrze jest przestrzegać zasady „rozdzielania obaw”. Tak więc przeniósłbym logikę analizowania XML do jakiejś osobnej klasy, a nie do niektórych metod statycznych należących do twojej ModelDefklasy.

Poprawka

Podejrzewam, że jeśli masz metodę, poza ModelDefktórą tworzy się ModelDefz XML, będziesz musiał utworzyć instancję jakiejś dynamicznej tymczasowej struktury danych drzewa, zapełnić ją przez parsowanie XML, a następnie utworzyć nową ModelDefprzekazywanie tej struktury jako parametr konstruktora. Być może można to uznać za zastosowanie wzorca „konstruktora”. Jest bardzo blisko analogia między tym, co chcesz zrobić, a String& StringBuilderpair. Znalazłem jednak to pytanie, które wydaje się nie zgadzać, z powodów, które nie są dla mnie jasne: Stackoverflow - StringBuilder i Builder Pattern . Tak więc, aby uniknąć tutaj długiej debaty na temat tego, czy StringBuilderwzorzec „konstruktora” jest realizowany, czy nie, powiedziałbym, że można zainspirować się tym, jakStrungBuilder pracuje nad znalezieniem rozwiązania, które odpowiada Twoim potrzebom, i odkłada nazywanie go wzorem „Konstruktora”, dopóki ten drobiazg nie zostanie ustalony.

Zobacz to nowe pytanie: programiści SE: Czy „StringBuilder” jest aplikacją wzorca projektowego Builder?

Mike Nakis
źródło
3
@RichardLevasseur Po prostu pamiętam, że był to temat niepokoju i debaty wśród programistów C ++ na początku do połowy lat dziewięćdziesiątych. Jeśli spojrzysz na ten post: gotw.ca/gotw/066.htm , zobaczysz, że jest dość skomplikowany, a dość skomplikowane rzeczy bywają kontrowersyjne. Nie wiem na pewno, ale myślę, że na początku lat dziewięćdziesiątych część tych rzeczy nie była jeszcze standaryzowana. Ale przepraszam, nie mogę podać dobrego odniesienia.
Mike Nakis,
1
@Gurtz Myślę o takiej klasie jako specyficznych dla aplikacji „narzędziach xml”, ponieważ format pliku xml (lub struktura dokumentu) jest prawdopodobnie związany z konkretną aplikacją, którą tworzysz, niezależnie od jakichkolwiek możliwości ponownie użyj „ModelDef”.
Mike Nakis,
1
@Gurtz, więc prawdopodobnie stworzyłbym dla nich metody instancji głównej klasy „Application”, a jeśli jest to zbyt duży problem, to metody statyczne niektórych klas pomocniczych, w sposób bardzo podobny do tego, co sugerował Ross Patterson.
Mike Nakis,
1
@Gurtz przeprasza, że ​​wcześniej nie zajął się konkretnie podejściem „budowniczym”. Poprawiłem swoją odpowiedź.
Mike Nakis,
3
@Gurtz Jest to możliwe, ale poza naukową ciekawością nie ma to znaczenia. Nie daj się wciągnąć w „anty-wzór”. Wzory są tak naprawdę tylko nazwami, aby wygodnie opisywać popularne / przydatne techniki kodowania. Zrób to, co musisz zrobić, przyklep etykietę później, jeśli chcesz to opisać. Zupełnie dobrze jest wdrożyć coś, co „może w pewnym sensie przypomina wzorzec konstruktora”, o ile kod ma sens. Ucząc się nowych technik, rozsądnie jest skupiać się na wzorach, po prostu nie wpadnij w pułapkę myślenia, że ​​wszystko, co robisz, musi być nazwanym wzorcem.
Jason C
9

Podałeś już najlepsze powody, aby nie wykonywać tej pracy w ModelDefkonstruktorze:

  1. Nie ma nic „lekkiego” w parsowaniu dokumentu XML do drzewa węzłów.
  2. Nie ma nic oczywistego w ModelDeftym, co mówi, że można go utworzyć tylko z dokumentu XML.

To brzmi jak klasa powinna mieć różnorodne metody statyczne, takie jak ModelDef.FromXmlString(string xmlDocument), ModelDef.FromXmlDoc(XmlDoc parsedNodeTree), itd.

Ross Patterson
źródło
Dziękuję za odpowiedź! W odniesieniu do sugestii metod statycznych. Czy byłyby to statyczne fabryki, które tworzą instancję ModelDef (z różnych źródeł XML)? A może byliby odpowiedzialni za ładowanie już utworzonego obiektu ModelDef? Jeśli to drugie, martwię się o to, że obiekt będzie tylko częściowo zainicjowany (ponieważ ModelDef potrzebuje drzewa węzłów do pełnej inicjalizacji). Myśli?
Gurtz,
3
Przepraszam za włączenie się, ale tak, Ross ma na myśli statyczne metody fabryczne, które zwracają w pełni zbudowane instancje. Pełny prototyp byłby podobny public static ModelDef createFromXmlString( string xmlDocument ). Jest to dość powszechna praktyka. Czasami też to robię. Moja sugestia, że można również zrobić tylko konstruktorów jest standardowy typ reakcji kopalni w sytuacjach, gdy podejrzewam, że alternatywne metody zostają odrzucone jako „nie koszerne” bez powodu.
Mike Nakis,
1
@ Mike-Nakis, dziękuję za wyjaśnienie. Tak więc w tym przypadku statyczna metoda fabryczna zbuduje drzewo węzłów, a następnie przekaże je do konstruktora (ewentualnie prywatnego) ModelDef. Ma sens. Dzięki.
Gurtz,
@Gurtz Dokładnie.
Ross Patterson,
5

Słyszałem już tę „zasadę”. Z mojego doświadczenia jest to zarówno prawda, jak i fałsz.

W bardziej „klasycznej” orientacji obiektowej mówimy o obiektach enkapsulujących stan i zachowanie. Zatem konstruktor obiektów powinien upewnić się, że obiekt został zainicjowany do poprawnego stanu (i zasygnalizować błąd, jeśli podane argumenty nie powodują, że obiekt jest poprawny). Zapewnienie inicjalizacji obiektu do prawidłowego stanu na pewno brzmi dla mnie jak prawdziwa praca. I ten pomysł ma swoje zalety, jeśli masz obiekt, który pozwala tylko na inicjację do prawidłowego stanu za pośrednictwem konstruktora, a obiekt poprawnie hermetyzuje go tak, aby każda metoda zmieniająca stan sprawdzała również, czy nie zmienia stanu na coś złego ... wtedy ten obiekt w istocie gwarantuje, że jest „zawsze ważny”. To naprawdę fajna nieruchomość!

Problem pojawia się na ogół, gdy próbujemy wszystko rozbić na małe części w celu przetestowania i wyszydzenia. Ponieważ jeśli obiekt jest naprawdę poprawnie zamknięty, nie można tak naprawdę tam wejść i zastąpić FooBarService wyśmiewanym FooBarService, a ty (prawdopodobnie) nie możesz po prostu zmienić wartości nie chcąc dopasować się do twoich testów (lub może to zająć dużo więcej kodu niż proste zadanie).

W ten sposób otrzymujemy drugą „szkołę myślenia”, która jest SOLIDNA. W tej szkole myślenia najprawdopodobniej o wiele bardziej prawdą jest, że nie powinniśmy wykonywać prawdziwej pracy w konstruktorze. Kod SOLID jest często (ale nie zawsze) łatwiejszy do przetestowania. Ale trudniej jest też uzasadnić. Dzielimy nasz kod na małe obiekty z pojedynczą odpowiedzialnością, a zatem większość naszych obiektów nie obudowuje już ich stanu (i ogólnie zawiera albo stan, albo zachowanie). Kod sprawdzania poprawności jest zwykle wyodrębniany do klasy walidatora i przechowywany osobno od stanu. Ale teraz straciliśmy spójność, nie możemy już być pewni, że nasze przedmioty są ważne, kiedy je otrzymamy i są całkowiciena pewno musimy zawsze potwierdzić, że warunki, które naszym zdaniem dotyczą obiektu, są prawdziwe, zanim spróbujemy coś z nim zrobić. (Oczywiście, ogólnie rzecz biorąc, sprawdzasz poprawność na jednej warstwie, a następnie zakładasz, że obiekt jest poprawny na niższych warstwach.) Ale łatwiej to przetestować!

Więc kto ma rację?

Tak naprawdę nikt. Obie szkoły myślenia mają swoje zalety. Obecnie SOLID jest wściekły i wszyscy mówią o SRP i Open / Closed i całym tym jazzie. Ale tylko dlatego, że coś jest popularne, nie oznacza, że ​​jest to właściwy wybór dla każdej aplikacji. To zależy. Jeśli pracujesz w bazie kodu, która ściśle przestrzega zasad SOLID, to tak, prawdziwa praca w konstruktorze jest prawdopodobnie złym pomysłem. Ale w przeciwnym razie spójrz na sytuację i spróbuj wykorzystać swój osąd. Jakie właściwości zyskuje Twój obiekt podczas pracy w konstruktorze, jakie właściwości traci ? Jak dobrze pasuje do ogólnej architektury Twojej aplikacji?

Prawdziwa praca w konstruktorze nie jest antypatternem, może być wręcz przeciwnie, jeśli jest używana we właściwych miejscach. Ale powinien być udokumentowany w jasny sposób (wraz z ewentualnymi wyjątkami) i jak w przypadku każdej decyzji projektowej - powinien pasować do ogólnego stylu używanego w bieżącej bazie kodu.

wasatz
źródło
To fantastyczna odpowiedź.
jrahhali
0

Z tą zasadą wiąże się fundamentalny problem i to właśnie stanowi „prawdziwą pracę”?

Z oryginalnego artykułu zamieszczonego w pytaniu widać, że autor próbuje zdefiniować, czym jest „prawdziwa praca”, ale ma ona poważne wady. Aby praktyka była dobra, musi być dobrze zdefiniowaną zasadą. Rozumiem przez to, że jeśli chodzi o inżynierię oprogramowania, pomysł powinien być przenośny (niezależnie od języka), przetestowany i sprawdzony. Większość argumentów zawartych w tym artykule nie spełnia tych pierwszych kryteriów. Oto kilka wskaźników, o których autor wspomina w tym artykule, co stanowi „prawdziwą pracę” i dlaczego nie są złymi definicjami.

Zastosowanie newsłowa kluczowego . Ta definicja jest zasadniczo błędna, ponieważ jest specyficzna dla domeny. Niektóre języki nie używają newsłowa kluczowego. Ostatecznie sugeruje, że nie powinno to być budowanie innych obiektów. Jednak w wielu językach nawet najbardziej podstawowe wartości same w sobie są obiektami. Tak więc każda wartość przypisana w konstruktorze również konstruuje nowy obiekt. To sprawia, że ​​ta idea ogranicza się do niektórych języków i jest złym wskaźnikiem tego, co stanowi „prawdziwą pracę”.

Obiekt nie został w pełni zainicjowany po zakończeniu konstruktora . To dobra zasada, ale jest także sprzeczna z kilkoma innymi zasadami wymienionymi w tym artykule. Dobrym przykładem tego, jak może to zaprzeczać innym, jest pytanie, które mnie tu przywiodło. W tym pytaniu ktoś jest zaniepokojony użyciem sortmetody w konstruktorze z powodu tej zasady, która wydaje się być JavaScript. W tym przykładzie osoba tworzyła obiekt, który reprezentował posortowaną listę innych obiektów. Na potrzeby dyskusji wyobraź sobie, że mieliśmy nieposortowaną listę obiektów i potrzebowaliśmy nowego obiektu do reprezentowania posortowanej listy. Potrzebujemy tego nowego obiektu, ponieważ pewna część naszego oprogramowania oczekuje posortowanej listy i pozwala wywołać ten obiektSortedList. Ten nowy obiekt akceptuje nieposortowaną listę, a wynikowy obiekt powinien reprezentować teraz posortowaną listę obiektów. Gdybyśmy mieli przestrzegać innych reguł wymienionych w tym dokumencie, a mianowicie żadnych statycznych wywołań metod, żadnych struktur przepływu sterowania, niczego więcej niż przypisania, wówczas wynikowy obiekt nie zostałby skonstruowany w prawidłowym stanie, łamiąc drugą zasadę pełnej inicjalizacji po zakończeniu konstruktora. Aby to naprawić, musielibyśmy wykonać podstawową pracę, aby posortować nieposortowaną listę w konstruktorze. Takie postępowanie spowodowałoby złamanie 3 innych zasad, przez co pozostałe zasady byłyby nieistotne.

Ostatecznie ta zasada nie wykonywania „prawdziwej pracy” w konstruktorze jest źle zdefiniowana i błędna. Próba zdefiniowania, co „prawdziwa praca” jest ćwiczeniem daremności. Najlepszą regułą w tym artykule jest to, że po zakończeniu konstruktora należy go w pełni zainicjować. Istnieje mnóstwo innych najlepszych praktyk, które ograniczałyby ilość pracy wykonanej w konstruktorze. Większość z nich można podsumować w zasadach SOLID, a te same zasady nie uniemożliwiłyby ci wykonywania pracy w konstruktorze.

PS. Czuję się zobowiązany do powiedzenia, że ​​chociaż twierdzę tutaj, że nie ma nic złego w wykonywaniu jakiejś pracy w konstruktorze, nie jest to również miejsce do wykonywania wielu prac. SRP sugeruje, że konstruktor powinien wykonać tylko tyle pracy, aby był poprawny. Jeśli twój konstruktor ma zbyt wiele wierszy kodu (bardzo subiektywnie wiem), prawdopodobnie narusza tę zasadę i prawdopodobnie mógłby zostać podzielony na mniejsze, lepiej zdefiniowane metody i obiekty.

zquintana
źródło