Przekazywanie obiektu do metody, która go zmienia, czy jest to wspólny (anty-) wzorzec?

Czytam o typowych zapachach kodu w książce Martin Fowler Refactoring . W tym kontekście zastanawiałem się nad wzorem, który widzę w bazie kodu i czy można obiektywnie uznać go za anty-wzór.

Wzorzec to taki, w którym obiekt jest przekazywany jako argument do jednej lub więcej metod, z których wszystkie zmieniają stan obiektu, ale żadna z nich nie zwraca obiektu. Opiera się więc na charakterze referencyjnym (w tym przypadku) C # / .NET.

var something = new Thing();
// ...
Foo(something);
int result = Bar(something, 42);
Baz(something);

Uważam, że (zwłaszcza gdy metody nie są odpowiednio nazwane) muszę przyjrzeć się takim metodom, aby zrozumieć, czy stan obiektu się zmienił. To sprawia, że ​​rozumienie kodu jest bardziej złożone, ponieważ muszę śledzić wiele poziomów stosu wywołań.

Chciałbym zaproponować ulepszenie takiego kodu, aby zwracał inny (sklonowany) obiekt z nowym stanem lub cokolwiek, co jest potrzebne do zmiany obiektu w miejscu wywołania.

var something1 =  new Thing();
// ...

// Let's return a new instance of Thing
var something2 = Foo(something1);

// Let's use out param to 'return' other info about the operation
int result;
var something3 = Bar(something2, out result);

// If necessary, let's capture and make explicit complex changes
var changes = Baz(something3)
something3.Apply(changes);

Wydaje mi się, że pierwszy wzór jest wybierany na podstawie założeń

• że jest to mniej pracy lub wymaga mniej wierszy kodu
• że pozwala nam zarówno zmienić obiekt, jak i zwrócić inną informację
• że jest bardziej wydajny, ponieważ mamy mniej przypadków Thing.

Ilustruję alternatywę, ale aby ją zaproponować, trzeba argumentować przeciwko oryginalnemu rozwiązaniu. Jakie argumenty można przedstawić, aby stwierdzić, że oryginalne rozwiązanie jest anty-wzorcem?

A co, jeśli w ogóle, jest nie tak z moim alternatywnym rozwiązaniem?

Tak, oryginalne rozwiązanie jest anty-wzorcem z powodów, które opisujesz: utrudnia rozumowanie o tym, co się dzieje, obiekt nie jest odpowiedzialny za swój własny stan / implementację (zerwanie enkapsulacji). Dodałbym również, że wszystkie te zmiany stanu są niejawnymi umowami metody, co czyni tę metodę kruchą w obliczu zmieniających się wymagań.

To powiedziawszy, twoje rozwiązanie ma swoje wady, z których najbardziej oczywistym jest to, że klonowanie obiektów nie jest świetne. W przypadku dużych obiektów może być wolny. Może to prowadzić do błędów, w których inne części kodu trzymają się starych odniesień (co prawdopodobnie ma miejsce w opisywanej bazie kodu). Uczynienie tych obiektów niezmiennymi rozwiązuje przynajmniej kilka z tych problemów, ale jest bardziej drastyczną zmianą.

O ile obiekty nie są małe i nieco przejściowe (co czyni ich dobrymi kandydatami na niezmienność), byłbym skłonny po prostu przenieść więcej przejścia stanu do samych obiektów. To pozwala ukryć szczegóły implementacji tych przejść i ustalić bardziej rygorystyczne wymagania dotyczące tego, kto / co / gdzie te zmiany stanu występują.

gdy metody nie są odpowiednio nazwane

Właściwie, to jest prawdziwe zapachy kodu. Jeśli masz obiekt podlegający zmianom, udostępnia metody zmiany jego stanu. Jeśli masz wywołanie takiej metody osadzonej w zadaniu kilku instrukcji, dobrze jest przefiltrować to zadanie do własnej metody - co pozostawia dokładną opisaną sytuację. Ale jeśli nie masz nazw metod takich jak Fooi Bar, ale metody, które wyjaśniają, że zmieniają obiekt, nie widzę tutaj problemu. Myśleć o

void AddMessageToLog(Logger logger, string msg)
{
    //...
}

lub

void StripInvalidCharsFromName(Person p)
{
// ...
}

lub

void AddValueToRepo(Repository repo,int val)
{
// ...
}

lub

void TransferMoneyBetweenAccounts(Account source, Account destination, decimal amount)
{
// ...
}

lub coś w tym rodzaju - nie widzę tu powodu, aby zwracać sklonowany obiekt dla tych metod, a także nie ma powodu, aby sprawdzać ich implementację, aby zrozumieć, że zmienią one stan przekazywanego obiektu.

Jeśli nie chcesz efektów ubocznych, spraw, aby twoje obiekty były niezmienne, wymusi metody takie jak powyższe, aby zwrócić zmieniony (sklonowany) obiekt bez zmiany oryginalnego.

Tak, zobacz http://codebetter.com/matthewpodwysocki/2008/04/30/side-effecting-functions-are-code-smells/, gdzie znajduje się jeden z wielu przykładów ludzi wskazujących, że nieoczekiwane skutki uboczne są złe.

Zasadniczo podstawową zasadą jest to, że oprogramowanie jest zbudowane warstwowo, a każda warstwa powinna przedstawiać następną możliwie najczystszą abstrakcję. A czysta abstrakcja to taka, w której musisz mieć jak najmniej na uwadze, aby z niej korzystać. Nazywa się to modułowością i dotyczy wszystkiego - od pojedynczych funkcji po protokoły sieciowe.

Przede wszystkim nie zależy to od „natury referencyjnej”, lecz od obiektów będących zmiennymi typami referencyjnymi. W językach niefunkcjonalnych prawie zawsze tak będzie.

Po drugie, to, czy jest to problem, czy nie, zależy zarówno od obiektu, jak i od tego, jak ściśle zmiany w różnych procedurach są ze sobą powiązane - jeśli nie dokonasz zmiany w Foo i spowoduje to awarię Bar, to problem. Niekoniecznie zapach kodu, ale jest to problem z Foo, Bar lub Coś (prawdopodobnie Bar tak jak powinien sprawdzać jego wejście, ale może to być Coś w niewłaściwym stanie, któremu powinien zapobiegać).

Nie powiedziałbym, że wznosi się do poziomu anty-wzoru, ale raczej coś, o czym należy pamiętać.

Twierdziłbym, że nie ma różnicy między A.Do(Something)modyfikowaniem somethinga something.Do()modyfikowaniem something. W obu przypadkach powinno być jasne z nazwy wywoływanej metody, która somethingzostanie zmodyfikowana. Jeśli nie jest jasne z nazwy metody, niezależnie od tego, czy somethingjest to parametr this, czy część środowiska, nie należy go modyfikować.

Myślę, że w niektórych sytuacjach można zmienić stan obiektu. Na przykład mam listę użytkowników i chcę zastosować różne filtry do listy przed zwróceniem jej do klienta.

var users = Dependency.Resolve<IGetUsersQuery>().GetAll();

var excludeAdminUsersFilter = new ExcludeAdminUsersFilter();
var filterByAnotherCriteria = new AnotherCriteriaFilter();

excludeAdminUsersFilter.Apply(users);
filterByAnotherCriteria.Apply(users); 

I tak, możesz uczynić to ładnym, przenosząc filtrowanie na inną metodę, dzięki czemu uzyskasz coś w stylu:

var users = Dependency.Resolve<IGetUsersQuery>().GetAll();
Filter(users);

Gdzie Filter(users)można wykonać powyższe filtry.

Nie pamiętam, gdzie dokładnie się z tym spotkałem, ale myślę, że nazywano to potokiem filtrującym.

Nie jestem pewien, czy proponowane nowe rozwiązanie (kopiowania obiektów) jest wzorem. Problemem, jak wskazałeś, jest zła nomenklatura funkcji.

Powiedzmy, że piszę złożoną operację matematyczną jako funkcję f () . I udokumentować, że f () jest funkcją, która odwzorowuje NXNsię N, a algorytm za nim. Jeśli funkcja jest nazwana niewłaściwie, nieudokumentowana i nie ma towarzyszących jej przypadków testowych, musisz zrozumieć kod, w którym to przypadku kod jest bezużyteczny.

O twoim rozwiązaniu, kilka spostrzeżeń:

• Aplikacje są projektowane z różnych aspektów: gdy obiekt jest używany tylko do przechowywania wartości lub jest przekazywany przez granice komponentów, mądrze jest zmienić wewnętrzne elementy obiektu na zewnątrz, zamiast wypełniać go szczegółami dotyczącymi zmiany.
• Klonowanie obiektów prowadzić do wzdęcia zapotrzebowanie na pamięć, a w wielu przypadkach prowadzi do istnienia równoważnych obiektów niezgodnych stanów ( Xstała Ypo f(), ale Xto faktycznie Y,) i ewentualnie niezgodność czasową.

Problem, który próbujesz rozwiązać, jest poprawny; jednak nawet przy ogromnej inżynierii, problem jest omijany, a nie rozwiązywany.