Czy modyfikowanie przychodzącego parametru jest antypatrakiem? [Zamknięte]

60

Programuję w Javie i zawsze tworzę konwertery w taki sposób:

public OtherObject MyObject2OtherObject(MyObject mo){
    ... Do the conversion
    return otherObject;
}

W nowym miejscu pracy wzór jest następujący:

public void MyObject2OtherObject(MyObject mo, OtherObject oo){
    ... Do the conversion
}

Dla mnie jest to trochę śmierdzące, ponieważ przyzwyczaiłem się nie zmieniać przychodzących parametrów. Czy ta przychodząca zmiana parametru jest antypatternem, czy jest w porządku? Czy ma jakieś poważne wady?

CsBalazsHungary
źródło
4
Właściwa odpowiedź na to pytanie będzie zależała od języka, ponieważ te, w których parametry przekazywania wartości skutecznie przekształcają je w zmienne lokalne.
Blrfl
1
Drugi wzór jest czasem wykorzystywany jako miara wydajności do ponownego wykorzystania obiektów. Zakładając, że oojest to obiekt przekazywany do metody, a nie wskaźnik ustawiony na nowy obiekt. Czy tak jest w tym przypadku? Jeśli jest to Java, prawdopodobnie tak jest, jeśli nie jest to C ++
Richard Tingle
3
To wygląda na przedwczesną optymalizację, tak. Radzę ogólnie używać pierwszego formularza, ale jeśli napotkasz wąskie gardło wydajności, możesz użyć drugiego formularza z komentarzem, aby to uzasadnić . Komentarz uniemożliwiłby tobie i innym osobom patrzenie na ten kod i ponowne pojawienie się tego samego pytania.
Keen
2
Drugi wzorzec jest przydatny, gdy funkcja zwraca również wartość, na przykład sukces / porażka. Ale nie ma w tym nic „złego”. To naprawdę zależy od tego, gdzie chcesz utworzyć obiekt.
GrandmasterB
12
Nie nazwałbym funkcji implementujących te dwa różne wzorce w ten sam sposób.
Casey

Odpowiedzi:

70

To nie jest antypattern, to zła praktyka.

Różnica między antypatternem a zwykłą złą praktyką jest tutaj: definicja anty-wzorca .

Nowy styl pracy, który pokazujesz, to zła praktyka , czas szczątkowy lub czas przed OOP, zgodnie z Clean Code wuja Boba.

Argumenty są najbardziej naturalnie interpretowane jako dane wejściowe do funkcji.

Wszystko, co zmusza cię do sprawdzenia podpisu funkcji, jest równoważne podwójnemu podejściu. Jest to przerwa poznawcza i należy jej unikać. W czasach poprzedzających programowanie obiektowe czasami konieczne były argumenty wyjściowe. Jednak większość potrzeby argumentów wyjściowych znika w językach OO

Tulains Córdova
źródło
7
Nie rozumiem, dlaczego definicja, którą podałeś, wyklucza uznanie tego za anty-wzór.
Samuel Edwin Ward
7
Przypuszczam, że dzieje się tak, ponieważ uzasadnienie, że „oszczędzamy procesor / pamięć nie tworząc nowego obiektu, zamiast tego powinniśmy poddać recyklingowi jeden, który mamy i przekazać jako argument”, jest tak oczywiście błędne dla każdego z poważnym doświadczeniem w zakresie OOP, że mogłoby to nigdy nie stanowią wzorca - więc nie ma mowy, abyśmy uważali go za anty-wzorzec z definicji ...
vaxquis
1
Co z przypadkiem, gdy parametr wejściowy jest dużą kolekcją obiektów, które wymagają modyfikacji?
Steve Chambers
Chociaż wolę też opcję 1, co jeśli OtherObjectinterfejs? Tylko osoba dzwoniąca (miejmy nadzieję) wie, jaki konkretny typ chce.
user949300,
@ SteveChambers Myślę, że w tym przypadku projekt klasy lub metody jest zły i należy go zreorganizować, aby tego uniknąć.
piotr.wittchen
16

Cytując słynną książkę Roberta C. Martina „Clean Code”:

Należy unikać argumentów wyjściowych

Funkcje powinny mieć małą liczbę argumentów

Drugi wzorzec narusza obie reguły, szczególnie „argument wyjściowy”. W tym sensie jest gorszy niż pierwszy wzór.

claasz
źródło
1
Powiedziałbym, że narusza pierwszy, wiele argumentów oznacza niechlujny kod, ale myślę, że dwa argumenty są całkowicie w porządku. Myślę, że reguła dotycząca czystego kodu oznacza, że ​​jeśli potrzebujesz 5 lub 6 przychodzących danych, chcesz zrobić zbyt wiele za pomocą jednej metody, więc powinieneś zreformować, aby zwiększyć czytelność kodu i zakres OOP.
CsBalazsHungary
2
w każdym razie podejrzewam, że nie jest to surowe prawo, ale silna sugestia. Wiem, że to nie zadziała z typami pierwotnymi, jeśli jest to szeroko rozpowszechniony wzorzec w projekcie, junior wpadłby na pomysł, aby przekazać int i oczekiwać, że zostanie zmieniony jak obiekty.
CsBalazsHungary
27
Niezależnie od tego, jak poprawny może być Clean Code, nie sądzę, że jest to cenna odpowiedź bez wyjaśnienia, dlaczego należy tego unikać. Nie są przykazaniami schodzącymi na kamienną tabliczkę, zrozumienie jest ważne. Ta odpowiedź może zostać ulepszona poprzez przedstawienie streszczenia uzasadnienia w książce i odniesienie do rozdziału
Daenyth
3
Do diabła, jeśli jakiś facet napisał to w książce, musi to być dobra rada na każdą możliwą sytuację. Nie trzeba myśleć.
Casey
2
@emodendroket Ale koleś, to facet!
Pierre Arlaud
15

Drugi idiom może być szybszy, ponieważ program wywołujący może ponownie użyć jednej zmiennej w długiej pętli, zamiast każdej iteracji tworząc nową instancję.

Na ogół nie używałbym tego, ale np. w programowaniu gier ma swoje miejsce. Na przykład spójrz na wiele operacji Java3Mon Vector3f pozwala na przekazanie instancji, którą należy zmodyfikować i zwrócić jako wynik.

użytkownik470365
źródło
12
Cóż, mogę się zgodzić, czy to jest wąskie gardło, ale gdziekolwiek pracowałem, wąskie gardła to zwykle algorytmy o niskiej wydajności, nie klonujące ani nie tworzą obiektów. Wiem mniej o tworzeniu gier.
CsBalazsHungary
4
@CsBalazsHungary Uważam, że problemy związane z tworzeniem obiektów są zwykle związane z alokacją pamięci i odśmiecaniem pamięci, co prawdopodobnie stanowiłoby kolejny poziom wąskich gardeł po złożoności algorytmicznej (szczególnie w środowisku o ograniczonej pamięci, np. Smartfonach itp.) .
JAB
JMonkey jest również miejscem, w którym często to widziałem, ponieważ często ma krytyczne znaczenie dla wydajności. Jednak nigdy nie słyszałem, aby nazywał się „JavaMonkey”
Richard Tingle
3
@JAB: GC JVM jest dostrojony specjalnie dla obiektów efemerycznych. Duże ilości bardzo krótko żyjących obiektów są łatwe do zebrania, aw wielu przypadkach całe efemeryczne pokolenie można zebrać jednym ruchem wskaźnika.
Phoshi
2
w rzeczywistości, jeśli trzeba stworzyć obiekt , nie będzie on efektywny pod względem wydajności; jeśli blok kodu musi być mocno zoptymalizowany pod kątem szybkości, powinieneś użyć prymitywów i macierzy natywnych; z tego powodu uważam, że stwierdzenie zawarte w tej odpowiedzi nie ma zastosowania.
vaxquis
10

Nie sądzę, że są to dwa równoważne fragmenty kodu. W pierwszym przypadku musisz utworzyć otherObject. Możesz zmodyfikować istniejącą instancję w drugim. Oba mają swoje zastosowania. Zapach kodu wolałby jeden od drugiego.

Euforyk
źródło
Wiem, że nie są równoważne. Masz rację, upieramy się przy tych rzeczach, więc to by miało znaczenie. Więc mówisz, że żaden z nich nie jest antypatternem, to tylko kwestia przypadku użycia?
CsBalazsHungary
@CsBalazsHungary Powiedziałbym tak. Sądząc po małym kawałku kodu, który podałeś.
Euforia
Wydaje mi się, że staje się to poważniejszym problemem, jeśli masz przychodzący prymitywny parametr, który oczywiście nie zostanie zaktualizowany, tak jak napisał @claasz: należy go unikać, chyba że jest to konieczne.
CsBalazsHungary
1
@CsBalazsHungary W przypadku prymitywnego argumentu funkcja nawet nie działałaby. Masz więc gorszy problem niż zmiana argumentów.
Euforyczny
Powiedziałbym, że junior wpadłby w pułapkę, próbując uczynić typ pierwotny parametrem wyjściowym. Powiedziałbym więc, że pierwszy konwerter powinien być preferowany, jeśli to możliwe.
CsBalazsHungary
7

To naprawdę zależy od języka.

W Javie druga postać może być anty-wzorcem, ale niektóre języki traktują przekazywanie parametrów inaczej. Ada i VHDL na przykład, zamiast przejść obojętnie wartość lub przez odwołanie, parametry mogą mieć tryby in, outalbo in out.

Błędem jest zmodyfikowanie inparametru lub odczytanie outparametru, ale wszelkie zmiany in outparametru są przekazywane z powrotem do programu wywołującego.

Tak więc dwie formy w Adzie (są to również legalne VHDL)

function MyObject2OtherObject(mo : in MyObject) return OtherObject is
begin
    ... Do the conversion
    return otherObject;
end MyObject2OtherObject;

i

procedure MyObject2OtherObject(mo : in MyObject; oo : out OtherObject) is
begin
    ... Do the conversion
    oo := ... the conversion result;
end MyObject2OtherObject;

Oba mają swoje zastosowania; procedura może zwrócić wiele wartości w wielu Outparametrach, podczas gdy funkcja może zwrócić tylko jeden wynik. Ponieważ cel drugiego parametru jest jasno określony w deklaracji procedury, nie można wnieść sprzeciwu wobec tego formularza. Wolę tę funkcję ze względu na czytelność. ale będą przypadki, w których procedura jest lepsza, np. gdy obiekt wywołujący już utworzył obiekt.

Brian Drummond
źródło
3

Rzeczywiście wydaje się śmierdzący i bez większego kontekstu nie można tego powiedzieć na pewno. Mogą to być dwa powody, chociaż istnieją alternatywy dla obu.

Po pierwsze, jest to zwięzły sposób na implementację częściowej konwersji lub pozostawienie wyniku z wartościami domyślnymi, jeśli konwersja się nie powiedzie. Oznacza to, że możesz mieć to:

public void ConvertFoo(Foo from, Foo to) {
    if (can't convert) {
        return;
    }
    ...
}

Foo a;
Foo b = DefaultFoo();
ConvertFoo(a, b);
// If conversion fails, b is unchanged

Oczywiście zwykle odbywa się to za pomocą wyjątków. Jednak nawet jeśli z jakiegokolwiek powodu należy unikać wyjątków, istnieje na to lepszy sposób - wzorzec TryParse jest jedną z opcji.

Innym powodem może być czysta spójność, na przykład jest to część publicznego interfejsu API, w którym ta metoda jest używana do wszystkich funkcji konwersji z dowolnego powodu (takich jak inne funkcje konwersji posiadające wiele danych wyjściowych).

Java nie jest świetna w radzeniu sobie z wieloma wyjściami - nie może mieć parametrów tylko wyjściowych, takich jak niektóre języki, ani mieć wielu wartości zwracanych, jak inne - ale nawet można użyć obiektów zwracanych.

Powód spójności jest raczej kiepski, ale niestety może być najczęstszy.

  • Być może stylowi gliniarze w twoim miejscu pracy (lub twojej bazie kodu) pochodzą z środowisk innych niż Java i niechętnie się zmieniają.
  • Twój kod mógł być portem z języka, w którym ten styl jest bardziej idiomatyczny.
  • Twoja organizacja może być zmuszona do zachowania spójności interfejsu API w różnych językach, a był to styl o najniższym wspólnym mianowniku (to głupie, ale zdarza się nawet w Google ).
  • A może styl był bardziej sensowny w odległej przeszłości i przekształcił się w obecną formę (na przykład mógł to być wzór TryParse, ale jakiś dobrze intonowany poprzednik usunął wartość zwracaną po odkryciu, że nikt jej wcale nie sprawdził).
congusbongus
źródło
2

Zaletą drugiego wzorca jest to, że zmusza on osobę dzwoniącą do przejęcia odpowiedzialności za utworzony obiekt. Nie ma wątpliwości, czy metoda utworzyła obiekt, czy pobrała go z puli wielokrotnego użytku. Dzwoniący wie, że jest odpowiedzialny za czas życia i utylizację nowego obiektu.

Wady tej metody to:

  1. Nie może być użyty jako metoda fabryczna, osoba dzwoniąca musi znać dokładny jej podtyp OtherObjecti musi go wcześniej zbudować.
  2. Nie można go używać z obiektami, które wymagają parametrów w ich konstruktorze, jeśli te parametry pochodzą MyObject. OtherObjectmusi być możliwy do zbudowania bez wiedzy o nim MyObject.

Odpowiedź oparta jest na moim doświadczeniu w c #, mam nadzieję, że logika przełoży się na Javę.

Rotem
źródło
Myślę, że wraz ze wspomnianą odpowiedzialnością tworzy również „trudniejszy do zauważenia” cykl życia przedmiotów. W złożonym systemie metod wolałbym bezpośrednią modyfikację obiektu niż zacząć przeglądać wszystkie metody, które przekazujemy.
CsBalazsHungary
Tak myślałem. Prymitywny rodzaj wstrzykiwania zależności
Lyndon White
Kolejną zaletą jest to, że OtherObject jest abstrakcyjny lub stanowi interfejs. Funkcja nie ma pojęcia, który typ utworzyć, ale wywołujący może.
user949300
2

Biorąc pod uwagę luźną semantykę - myObjectToOtherObjectmoże również oznaczać, że przenosisz niektóre dane z pierwszego obiektu do drugiego lub że przekształcisz je całkowicie od nowa, druga metoda wydaje się bardziej odpowiednia.

Gdyby jednak nazwa metody to Convert(co powinno być, jeśli spojrzymy na część „... wykonaj konwersję”), powiedziałbym, że druga metoda nie miałaby żadnego sensu. Nie zamieniasz wartości na inną wartość, która już istnieje, IMO.

guillaume31
źródło