Czy jest jakiś powód, aby wykonywać całą pracę obiektu w konstruktorze?

49

Pozwolę sobie powiedzieć, że nie jest to mój kod ani kod moich współpracowników. Wiele lat temu, kiedy nasza firma była mniejsza, mieliśmy pewne projekty, które musieliśmy wykonać, do których nie mieliśmy możliwości, więc zostały zlecone na zewnątrz. Teraz nie mam nic przeciwko outsourcingowi ani ogólnie wykonawcom, ale baza kodów, którą stworzyli, to masa WTF. To powiedziawszy, działa (głównie), więc przypuszczam, że znajduje się w 10% najlepszych projektów outsourcingowych, jakie widziałem.

W miarę rozwoju naszej firmy staraliśmy się więcej rozwijać. Ten konkretny projekt wylądował mi na kolanach, więc go omijałem, sprzątałem, dodawałem testy itp.

Jest jeden wzór, który często powtarzam i wydaje się tak niesamowicie okropny, że zastanawiałem się, czy może jest jakiś powód i po prostu go nie widzę. Wzorzec jest obiektem bez publicznych metod lub elementów, tylko publicznym konstruktorem, który wykonuje całą pracę na obiekcie.

Na przykład (jeśli kod ma znaczenie w Javie, ale mam nadzieję, że będzie to bardziej ogólne pytanie):

public class Foo {
  private int bar;
  private String baz;

  public Foo(File f) {
    execute(f);
  }

  private void execute(File f) {
     // FTP the file to some hardcoded location, 
     // or parse the file and commit to the database, or whatever
  }
}

Jeśli zastanawiasz się, ten typ kodu jest często wywoływany w następujący sposób:

for(File f : someListOfFiles) {
   new Foo(f);
}

Teraz nauczono mnie dawno temu, że instancyjne obiekty w pętli są ogólnie złym pomysłem i że konstruktorzy powinni wykonać minimum pracy. Patrząc na ten kod, wygląda na to, że lepiej byłoby porzucić konstruktor i zrobić executepubliczną metodę statyczną.

Zapytałem wykonawcę, dlaczego zostało to zrobione w ten sposób, a otrzymałem odpowiedź: „Możemy to zmienić, jeśli chcesz”. Co nie było tak naprawdę pomocne.

W każdym razie, czy jest jakiś powód, aby zrobić coś takiego w jakimkolwiek języku programowania, czy to tylko kolejne zgłoszenie do Daily WTF?

Laska
źródło
18
Wygląda na to, że napisali go programiści, którzy wiedzą o public static void main(string[] args)obiektach i słyszeli o nich, a następnie próbowali je złączyć.
Andy Hunt
Jeśli nigdy więcej nie zadzwonisz, musisz to zrobić. Wiele współczesnych ram dla np. Wstrzykiwania zależności wymaga wykonania ziaren, ustawienia właściwości i następnie wywołania, a następnie nie można użyć tych ciężkich pracowników.
4
Powiązane pytanie: Masz problem z wykonaniem głównej pracy klasy w jej konstruktorze? . Ten jest jednak nieco inny, ponieważ utworzona instancja jest później używana.
śleske,
dobra lektura na temat Oddzielanie obiektu Użyj od jego konstrukcji
Songo,
Naprawdę nie chcę rozszerzać tego na pełną odpowiedź, ale powiem tylko: istnieją scenariusze, w których jest to prawdopodobnie do przyjęcia. Ten jest bardzo daleki od tego. W tym przypadku obiekt jest tworzony bez jego potrzeby. W przeciwieństwie do tego, jeśli tworzysz jakąś strukturę danych z pliku XML, zainicjowanie parsowania z konstruktora nie jest całkowicie okropne. W rzeczywistości w językach, które pozwalają na przeciążanie konstruktorów, to nic, za co cię
zabiłbym

Odpowiedzi:

60

Ok, schodząc w dół listy:

Dawno temu nauczono mnie, że instancyjne obiekty w pętli są ogólnie złym pomysłem

Nie w żadnym używanym przeze mnie języku.

W C dobrze jest zadeklarować zmienne z góry, ale różni się to od tego, co powiedziałeś. Może być nieco szybszy, jeśli zadeklarujesz obiekty powyżej pętli i użyjesz ich ponownie, ale istnieje wiele języków, w których ten wzrost prędkości będzie bez znaczenia (i prawdopodobnie niektóre kompilatory, które dokonują optymalizacji dla Ciebie :)).

Ogólnie, jeśli potrzebujesz obiektu w pętli, utwórz go.

konstruktorzy powinni wykonać minimum pracy

Konstruktory powinny utworzyć instancję pól obiektu i wykonać dowolną inną inicjalizację niezbędną do przygotowania obiektu do użycia. Ogólnie oznacza to, że konstruktory są małe, ale istnieją scenariusze, w których byłoby to znaczną ilość pracy.

czy kiedykolwiek jest jakiś powód, aby zrobić coś takiego, w jakimkolwiek języku programowania, czy to tylko kolejne zgłoszenie do Daily WTF?

To zgłoszenie do Daily WTF. Oczywiście istnieją gorsze rzeczy, które możesz zrobić z kodem. Problem polega na tym, że autor ma ogromne niezrozumienie, czym są klasy i jak z nich korzystać. W szczególności oto, co według mnie jest nie tak z tym kodem:

  • Niewłaściwe użycie klas: Klasa zasadniczo działa jak funkcja. Jak wspomniałeś, należy go albo zastąpić funkcją statyczną, albo funkcję należy po prostu zaimplementować w klasie, która ją wywołuje. Zależy od tego, co robi i gdzie jest używany.
  • Narzut wydajności: w zależności od języka tworzenie obiektu może być wolniejsze niż wywołanie funkcji.
  • Ogólne zamieszanie: Generalnie programiści mylą sposób korzystania z tego kodu. Bez jego użycia nikt nie wiedziałby, w jaki sposób autor zamierzał użyć tego kodu.
riwalk
źródło
Dzięki za twoją odpowiedź. W odniesieniu do „tworzenia instancji obiektów w pętli” jest to coś, o czym ostrzega wiele narzędzi do analizy statycznej, a także słyszałem o tym od niektórych profesorów college'ów. To prawda, że ​​może to być opóźnienie z lat 90., kiedy wydajność była większa. Oczywiście, jeśli potrzebujesz, zrób to, ale jestem zaskoczony słysząc przeciwny punkt widzenia. Dzięki za poszerzenie moich horyzontów.
Kane
8
Tworzenie obiektów w pętli jest niewielkim zapachem kodu; powinieneś spojrzeć na to i zapytać: „Czy naprawdę muszę wielokrotnie tworzyć ten obiekt?”. Jeśli tworzysz instancję tego samego obiektu z tymi samymi danymi i każesz mu zrobić to samo, zaoszczędzisz cykle (i unikniesz przeładowania pamięci) poprzez utworzenie instancji raz, a następnie tylko powtórzenie instrukcji z wszelkimi przyrostowymi zmianami, które mogą być konieczne do stanu obiektu . Jeśli jednak, na przykład, czytasz strumień danych z bazy danych lub innych danych wejściowych i tworzysz serię obiektów do przechowywania tych danych, z całą pewnością od razu się od nich odzywa.
KeithS,
3
Krótka wersja: Nie używaj instancji obiektów, jakby były funkcją.
Erik Reppen,
27

Dla mnie jest to trochę niespodzianka, gdy dostajesz obiekt, który wykonuje wiele pracy w konstruktorze, dlatego właśnie odradzam to (zasada najmniejszego zaskoczenia).

Próba dobrego przykładu :

Nie jestem pewien, czy to użycie jest anty-wzorzec, ale w C # widziałem kod, który był zgodny z (nieco wymyślony przykład):

using (ImpersonationContext administrativeContext = new ImpersonationContext(ADMIN_USER))
{
    // Perform actions here under the administrator's security context
}
// We're back to "normal" privileges here

W takim przypadku ImpersonationContextklasa wykonuje całą swoją pracę w konstruktorze i metodzie Dispose. Korzysta z instrukcji C # using, która jest dość dobrze rozumiana przez programistów C #, a tym samym gwarantuje, że konfiguracja występująca w konstruktorze zostanie wycofana, gdy znajdziemy się poza usinginstrukcją, nawet jeśli wystąpi wyjątek. Jest to prawdopodobnie najlepsze użycie, jakie widziałem w tym celu (tzn. „Praca” w konstruktorze), chociaż nadal nie jestem pewien, czy uważam to za „dobre”. W tym przypadku jest to dość oczywiste, funkcjonalne i pomocne.

Przykładem dobrego i podobnego wzorca może być wzorzec polecenia . Zdecydowanie nie wykonuje się tam logiki w konstruktorze, ale jest podobny w tym sensie, że cała klasa jest równoważna wywołaniu metody (w tym parametrom potrzebnym do wywołania metody). W ten sposób informacje wywołania metody mogą zostać przekształcone do postaci szeregowej lub przekazane do późniejszego użycia, co jest niezwykle przydatne. Być może twoi kontrahenci próbowali czegoś podobnego, chociaż bardzo w to wątpię.

Komentarze do twojego przykładu:

Powiedziałbym, że to bardzo wyraźny anty-wzór. Nie dodaje nic, jest sprzeczne z oczekiwaniami i wykonuje zbyt długie przetwarzanie w konstruktorze (FTP w konstruktorze? WTF rzeczywiście, chociaż myślę, że ludzie znani są z tego, że nazywają usługi sieciowe).

Próbuję znaleźć cytat, ale książka Grady Booch „Object Solutions” ma anegdotę na temat zespołu programistów C przechodzących na C ++. Najwyraźniej przeszli szkolenie i zarząd powiedział im, że absolutnie muszą robić rzeczy „zorientowane obiektowo”. Wprowadzony, aby dowiedzieć się, co jest nie tak, autor użył narzędzia metryki kodu i stwierdził, że średnia liczba metod na klasę wynosi dokładnie 1, i były odmianami wyrażenia „zrób to”. Muszę powiedzieć, że nigdy wcześniej nie spotkałem się z tym konkretnym problemem, ale najwyraźniej może to być symptom zmuszania ludzi do tworzenia kodu obiektowego, nie bardzo rozumiejąc, jak to zrobić i dlaczego.

Daniel B.
źródło
1
Dobrym przykładem jest przykład alokacji zasobów to Inicjalizacja . Jest to zalecany wzorzec w C ++, ponieważ znacznie ułatwia pisanie kodu bezpiecznego dla wyjątków. Nie wiem jednak, czy to przenosi się na C #. (W tym przypadku przydzielanym „zasobem” są uprawnienia administracyjne.)
zwolnij
@Zack Nigdy o tym nie myślałem, mimo że wiedziałem o RAII w C ++. W świecie C # jest to określane jako wzorzec jednorazowego użytku i jest bardzo zalecane dla wszystkiego, co ma (zwykle niezarządzane) zasoby do zwolnienia po okresie użytkowania. Główną różnicą w tym przykładzie jest to, że zwykle nie wykonuje się drogich operatorów w konstruktorze. Np. Metoda Open () SqlConnection nadal wymaga wywołania za konstruktorem.
Daniel B
14

Nie.

Jeśli cała praca zostanie wykonana w konstruktorze, oznacza to, że obiekt nigdy nie był potrzebny. Najprawdopodobniej wykonawca po prostu użył tego obiektu do modułowości. W takim przypadku klasa bez instancji z metodą statyczną uzyskałaby tę samą korzyść bez tworzenia zbędnych instancji obiektów.

Jest tylko jeden przypadek, w którym wykonanie całej pracy w konstruktorze obiektów jest dopuszczalne. Wtedy celem obiektu jest konkretnie reprezentowanie długoterminowej unikalnej tożsamości, a nie wykonywanie pracy. W takim przypadku obiekt byłby trzymany w pobliżu z powodów innych niż wykonywanie znaczącej pracy. Na przykład instancja singletonu.

Kevin A. Naudé
źródło
7

Z pewnością stworzyłem wiele klas, które wykonują wiele pracy, aby obliczyć coś w konstruktorze, ale obiekt jest niezmienny, więc udostępnia wyniki tego obliczenia za pomocą właściwości, a następnie nigdy nie robi nic innego. To normalna niezmienność w akcji.

Myślę, że dziwne jest tutaj umieszczenie kodu z efektami ubocznymi w konstruktorze. Budowanie obiektu i wyrzucanie go bez uzyskiwania dostępu do właściwości lub metod wygląda dziwnie (ale przynajmniej w tym przypadku jest całkiem oczywiste, że dzieje się coś innego).

Byłoby lepiej, gdyby funkcja została przeniesiona do metody publicznej, a następnie wprowadzono instancję klasy, a następnie kilkakrotnie wywołano metodę pętli for.

Scott Whitlock
źródło
4

Czy jest jakiś powód, aby wykonywać całą pracę obiektu w konstruktorze?

Nie.

  • Konstruktor nie powinien mieć żadnych skutków ubocznych.
    • Coś więcej niż inicjalizacja pola prywatnego powinna być postrzegana jako efekt uboczny.
    • Konstruktor z efektami ubocznymi łamie zasadę pojedynczej odpowiedzialności (SRP) i działa wbrew duchowi programowania obiektowego (OOP).
  • Konstruktor powinien być lekki i nigdy nie powinien zawieść.
    • Na przykład zawsze wzdrygam się, gdy widzę blok try-catch w konstruktorze. Konstruktor nie powinien zgłaszać wyjątków ani rejestrować błędów.

Można rozsądnie zakwestionować te wytyczne i powiedzieć: „Ale nie przestrzegam tych zasad, a mój kod działa dobrze!” Na to odpowiedziałbym: „Cóż, to może być prawda, dopóki tak nie będzie”.

  • Wyjątki i błędy wewnątrz konstruktora są bardzo nieoczekiwane. O ile nie zostanie im to nakazane, przyszli programiści nie będą skłonni otaczać tych wywołań konstruktora kodem obronnym.
  • Jeśli coś zawiedzie w produkcji, wygenerowany ślad stosu może być trudny do przeanalizowania. Początek śledzenia stosu może wskazywać na wywołanie konstruktora, ale wiele rzeczy dzieje się w konstruktorze i może nie wskazywać na faktyczne LOC, które nie powiodło się.
    • Przetwarzałem wiele śladów stosu .NET tam, gdzie tak było.
Jim G.
źródło
1
„O ile nie zostanie im to nakazane, przyszli programiści nie będą skłonni otaczać tych wywołań konstruktora kodem obronnym”. - Słuszna uwaga.
Graham
2

Wydaje mi się, że osoba robiąca to próbowała włamać się do ograniczeń Javy, które nie pozwalają na przekazywanie funkcji obywatelom pierwszej klasy. Ilekroć musisz przekazać funkcję, musisz owinąć ją wewnątrz klasy metodą podobną applylub podobną.

Sądzę więc, że po prostu użył skrótu i ​​użył bezpośrednio klasy jako zamiennika funkcji. Ilekroć chcesz wykonać funkcję, po prostu utwórz instancję klasy.

Zdecydowanie nie jest to dobry wzór, przynajmniej dlatego, że próbujemy to rozgryźć, ale myślę, że może to być najmniej szczegółowy sposób na zrobienie czegoś podobnego do programowania funkcjonalnego w Javie.

Andrea
źródło
3
Jestem pewien, że ten konkretny programista nigdy nie słyszał wyrażenia „programowanie funkcjonalne”.
Kane
Nie sądzę, że programista próbował stworzyć abstrakcję zamknięcia. Odpowiednik Java wymagałby abstrakcyjnej metody (planowania ewentualnego) polimorfizmu. Tutaj nie ma na to dowodów.
Kevin A. Naudé
1

Dla mnie podstawową zasadą jest nie robienie niczego w konstruktorze, z wyjątkiem inicjowania zmiennych składowych. Pierwszym powodem tego jest to, że pomaga przestrzegać zasady SRP, ponieważ zwykle długi proces inicjalizacji wskazuje, że klasa robi więcej niż powinna, a inicjalizacja powinna odbywać się gdzieś poza klasą. Drugim powodem jest to, że w ten sposób przekazywane są tylko niezbędne parametry, dzięki czemu tworzony jest mniej sprzężony kod. Konstruktor ze skomplikowaną inicjalizacją zwykle potrzebuje parametrów, które są używane tylko do konstruowania innego obiektu, ale które nie są używane przez klasę oryginalną.

rmaruszewski
źródło
1

Idę tutaj pod prąd - w językach, w których wszystko musi być w jakiejś klasie ORAZ nie możesz mieć klasy statycznej, możesz natknąć się na sytuację, w której masz izolowaną funkcjonalność, która musi być umieścić gdzieś i nie pasuje do niczego innego. Twoje wybory to klasa użyteczności, która obejmuje różne niepowiązane ze sobą elementy funkcjonalności, lub klasa, która robi w zasadzie tylko to jedno.

Jeśli masz tylko jeden taki bit, to twoja klasa statyczna jest twoją specyficzną klasą. Więc masz konstruktora, który wykonuje całą pracę, lub pojedynczą funkcję, która wykonuje całą pracę. Zaletą robienia wszystkiego w konstruktorze jest to, że dzieje się to tylko raz, pozwala korzystać z prywatnych pól, właściwości i metod, nie martwiąc się o ich ponowne użycie lub wątkowanie. Jeśli masz konstruktor / metodę, możesz ulec pokusie, aby pozwolić na przekazanie parametrów do pojedynczej metody, ale jeśli używasz prywatnych pól lub właściwości, które wprowadzają potencjalne problemy z wątkami.

Nawet w przypadku klasy użyteczności większość ludzi nie uważa, że ​​zagnieżdżono klasy statyczne (co może nie być możliwe w zależności od języka).

Zasadniczo jest to stosunkowo zrozumiały sposób izolowania zachowania od reszty systemu, w granicach dozwolonych przez język, przy jednoczesnym umożliwieniu korzystania z jak największej ilości języka.

Szczerze mówiąc, odpowiedziałbym dużo, tak jak zrobił to twój kontrahent, to drobna korekta, aby przekształcić go w dwa połączenia, nie ma aż tak wiele, aby polecić jedno nad drugim. Jakie są wady / zalety, są prawdopodobnie bardziej hipotetyczne niż rzeczywiste, po co próbować uzasadniać decyzję, gdy nie ma ona znaczenia i prawdopodobnie nie była to decyzja tak ważna, jak działanie domyślne?

jmoreno
źródło
1

Istnieją wzorce powszechne w językach, w których funkcje i klasy są znacznie bardziej takie same, jak w Pythonie, w którym jeden obiekt zasadniczo używa funkcji, która ma zbyt wiele efektów ubocznych lub zwraca wiele nazwanych obiektów.

W tym przypadku większość, jeśli nie cała praca, może zostać wykonana w konstruktorze, ponieważ sam obiekt jest tylko opakowaniem wokół pojedynczego pakietu pracy i nie ma żadnego powodu, aby go umieszczać w osobnej funkcji. Coś jak

var parser = XMLParser()
var x = parser.parse(string)
string a = x.LookupTag(s)

naprawdę nie jest lepszy niż

 var x = ParsedXML(string)
 string a = x.LookupTag(s)

Zamiast bezpośrednio wywoływać efekty uboczne, możesz wywołać obiekt w celu wymuszenia efektu ubocznego na sygnał, co zapewnia lepszą widoczność. Jeśli mam mieć zły efekt uboczny na obiekcie, mogę wykonać całą moją pracę, a następnie przejść obok obiektu, na który źle wpłynę i wyrządzę mu obrażenia. Identyfikacja obiektu i izolowanie wywołania w ten sposób jest wyraźniejsze niż przekazywanie wielu takich obiektów do funkcji jednocześnie.

x.UpdateView(v)

Można wprowadzić wiele wartości zwracanych za pośrednictwem akcesoriów na obiekcie. Cała praca została wykonana, ale chcę to wszystko w kontekście i tak naprawdę nie chcę przekazywać wielu referencji do mojej funkcji.

var x = new ComputeStatistics(inputFile)
int max = x.MaxOptions;
int mean = x.AverageOption;
int sd = x.StandardDeviation;
int k = x.Kurtosis;
...

Więc jako wspólny programista Python / C # nie wydaje mi się to aż tak dziwne.

Jon Jay Obermark
źródło
Muszę wyjaśnić, że podany przykład nadal wprowadza w błąd. W przykładzie bez akcesoriów i bez wartości zwracanej jest to prawdopodobnie śladowe. Być może dostarczony oryginał zwraca debugowanie lub coś takiego.
Jon Jay Obermark
1

Przychodzi mi na myśl jeden przypadek, w którym konstruktor / destruktor wykonuje całą pracę:

Zamki Zwykle nigdy nie wchodzisz w interakcje z zamkiem w trakcie jego życia. Używanie architektury przez C # jest dobre do obsługi takich przypadków bez wyraźnego odwoływania się do destruktora. Jednak nie wszystkie blokady są realizowane w ten sposób.

Napisałem także fragment kodu, który był tylko konstruktorem, aby załatać błąd biblioteki wykonawczej. (Właściwie naprawienie kodu spowodowałoby inne problemy.) Nie było destruktora, ponieważ nie było potrzeby cofania poprawki.

Loren Pechtel
źródło
-1

Zgadzam się z AndyBursh. Wydaje się, że jest to kwestia braku wiedzy.

Jednak ważne jest, aby wspomnieć o frameworkach takich jak NHibernate, które wymagają tylko kodowania w konstruktorze (podczas tworzenia klas map). Jednak nie jest to ograniczenie frameworka, to tylko to, czego potrzebujesz. Jeśli chodzi o refleksję, konstruktor jest jedyną metodą, która zawsze będzie istnieć (chociaż może być prywatna) i może to być pomocne, jeśli trzeba wywołać metodę klasy dynamicznie.

fabiopagoti
źródło
-4

„Już dawno temu nauczono mnie, że instancyjne obiekty w pętli są ogólnie złym pomysłem”

Tak, może pamiętasz, dlaczego potrzebujemy StringBuilder.

Istnieją dwie szkoły Java.

Konstruktor został promowany przez pierwszy , który uczy nas ponownego użycia (ponieważ dzielenie = tożsamość do wydajności). Jednak na początku 2000 roku zacząłem czytać, że tworzenie obiektów w Javie jest tak tanie (w przeciwieństwie do C ++), a GC jest tak wydajne, że ponowne użycie jest bezwartościowe, a liczy się tylko wydajność programisty. Aby zwiększyć produktywność, musi napisać zarządzalny kod, co oznacza modularyzację (czyli zawężenie zakresu). Więc,

to jest złe

byte[] array = new byte[kilos or megas]
for_loop:
  use(array)

to jest dobre.

for_loop:
  byte[] array = new byte[kilos or megas]
  use(array)

Zadałem to pytanie, gdy istniało forum.java.sun, a ludzie zgodzili się, że woleliby, aby tablica była możliwie jak najbardziej wąska.

Współczesny programista Java nigdy nie waha się zadeklarować nowej klasy lub stworzyć obiekt. Jest do tego zachęcany. Java daje powód, aby to zrobić, z pomysłem, że „wszystko jest przedmiotem” i tanim stworzeniem.

Co więcej, nowoczesny styl programowania korporacyjnego polega na tym, że kiedy trzeba wykonać akcję, tworzy się specjalną klasę (lub jeszcze lepiej framework), która wykona tę prostą akcję. Dobre IDE Java, które tutaj pomagają. Generują tony bzdur automatycznie podczas oddychania.

Myślę, że twoim obiektom brakuje wzorca Builder lub Factory. Tworzenie instancji bezpośrednio przez konstruktora nie jest przyzwoite. Zalecana jest specjalna klasa fabryczna dla każdego obiektu.

Teraz, gdy w grę wchodzi programowanie funkcjonalne, tworzenie obiektów staje się jeszcze prostsze. Będziesz tworzyć obiekty na każdym kroku jako oddychanie, nawet tego nie zauważając. Oczekuję więc, że Twój kod stanie się jeszcze bardziej „wydajny” w nowej erze

„nie rób nic w swoim konstruktorze. To jest tylko inicjalizacja”

Osobiście nie widzę powodu w wytycznych. Uważam to za kolejną metodę. Faktoring kodu jest konieczny tylko wtedy, gdy możesz go udostępnić.

Val
źródło
3
wyjaśnienie nie jest dobrze skonstruowane i trudne do naśladowania. Po prostu chodzisz wszędzie z twierdzeniem, które nie ma linków do kontekstu.
Nowa Aleksandria,
Naprawdę sporne jest stwierdzenie: „Konstruktorzy powinni tworzyć instancje pól obiektu i dokonywać wszelkich innych inicjalizacji niezbędnych do przygotowania obiektu do użycia”. Powiedz o tym najpopularniejszemu współautorowi.
Val