Czy to zapach kodu, jeśli często tworzysz obiekt, aby wywołać na nim metodę

14

Odziedziczyłem bazę kodu, w której jest dużo kodu, który wygląda mniej więcej tak:

SomeDataAdapter sda = new SomeDataAdapter();
sda.UpdateData(DataTable updateData);

I wtedy sda już nigdy nie jest używane.

Czy to zapach kodu wskazujący, że te metody powinny być metodami klasy statycznej?

Shane Wealti
źródło
Czy te klasy implementują jakieś interfejsy?
Reactgular,
7
Dzień po refaktoryzacji do metod statycznych będzie dniem, w którym chcesz uruchomić testy jednostkowe z fałszywą wersją adaptera danych.
Mike
22
@ Mike: Dlaczego miałbyś kiedykolwiek kpić z metody statycznej? Okropnie męczy mnie ten błędny argument, że metod statycznych nie da się przetestować. Jeśli twoje metody statyczne utrzymują stan lub powodują skutki uboczne, to twoja wina, a nie wina twojej metodologii testowania.
Robert Harvey
9
Moim zdaniem jest to zapach języka pokazujący słabość analizy „wszystko musi być klasą”.
Ben
7
@RobertHarvey: może być konieczne wyśmiewanie metody statycznej z tego samego powodu, z którego wyśmiewasz inną metodę: wywoływanie podczas testowania jednostkowego jest zbyt drogie.
kevin cline

Odpowiedzi:

1

Uznałbym to za zapach architektury w tym, że UpdateData prawdopodobnie powinien należeć do klasy „service”.

Gdzie dane to Apple. Gdzie AppleAdapter jest usługą / klasą inteligencji biznesowej. Gdzie AppleService to odwołanie Singleton do AppleAdapter, który istnieje poza bieżącą metodą.

private static volatile AppleAdapter _appleService = null;
private static object _appleServiceLock = new object();
private AppleAdapter AppleService
{
    get
    {
        if (_appleService == null)
        {
            lock (_appleServiceLock)
            {
                if (_appleService == null)
                    _appleService = new AppleAdapter();
            }
        }
        return _appleService;
    }
}

public SomeAppleRelatedMethod(Apple apple)
{
    AppleService.UpdateData(apple);
}

Nie sądzę, że to, co robisz, jest niekoniecznie złe, ale jeśli SomeDataAdapter rzeczywiście reprezentuje jakąś bezpaństwową usługę biznesową, wówczas najlepszym rozwiązaniem będzie singleton. Mam nadzieję, że to pomaga! Podany przykład jest fantazyjnym sposobem na zapewnienie braku rywalizacji _appleService, jeśli zdarzyło się, że jest ona zarówno pusta, jak i dostępna w tym samym czasie przez dwa lub więcej wątków.

Wiesz co? Jeśli SomeDataAdapter jest ADO IDbDataAdapter (którym prawie na pewno jest), zignoruj ​​całą odpowiedź!

: P

Nie mam uprawnień, aby dodać komentarz do pierwotnego pytania, ale jeśli możesz określić, gdzie istnieje ten kod.

Jeśli ten kod reprezentuje niestandardową implementację IDbDataAdapter, a UpdateData tworzy IDbConnection, IDbCommand i łączy to wszystko za kulisami, to nie, nie uważałbym, że kod pachnie, ponieważ teraz mówimy o strumieniach i innych rzeczy, które należy usunąć, gdy skończymy ich używać.

Andrew Hoffman
źródło
12
Wsparcie! Tonę we wzorcach oprogramowania.
Robert Harvey
4
... lub wstrzykiwany jako zależność. ;)
Rob
12
Bałaganowanie singletonów i podwójnie sprawdzane blokowanie, aby uzyskać coś równoważnego prostej funkcji / procedurze, jest dla mnie znacznie większym zapachem kodu!
Ben
3

Myślę, że ten problem sięga jeszcze głębiej. Nawet jeśli przekształcisz go w metodę statyczną, otrzymasz kod, w którym wywołujesz pojedynczą metodę statyczną w dowolnym miejscu. Który moim zdaniem sam w sobie pachnie kodem. Może to oznaczać, że brakuje Ci ważnej abstrakcji. Być może chcesz utworzyć kod, który wykona wstępne i końcowe przetwarzanie, umożliwiając jednocześnie zmianę tego, co dzieje się pomiędzy nimi. To wstępne i końcowe przetwarzanie będzie zawierać wspólne wezwania, a pomiędzy nimi będzie zależeć od konkretnego wdrożenia.

Euforyk
źródło
Zgadzam się, że jest o wiele głębszy problem. Staram się stopniowo to ulepszać bez pełnego przeprojektowania architektury, ale wydaje mi się, że to, co myślałem, nie jest dobrym sposobem na zrobienie tego.
Shane Wealti
1

Rodzaj. Zazwyczaj oznacza to, że chcesz przekazać funkcję, a wybrany język ci na to nie pozwala. Jest to trochę niezdarne i nieeleganckie, ale jeśli utkniesz w języku bez pierwszorzędnych funkcji, niewiele możesz zrobić.

Jeśli żaden z tych obiektów nie zostanie przekazany jako argument do innych funkcji, możesz zamienić je w metody statyczne i nic by się nie zmieniło.

Doval
źródło
Może to również oznaczać, że chcesz, aby metoda statyczna zwróciła zmodyfikowaną wersję (lub zmodyfikowaną kopię) przekazywanego do niej obiektu.
Robert Harvey
2
„jeśli utkniesz w języku bez funkcji pierwszej klasy”: nie ma różnicy między obiektem za pomocą tylko jednej metody a funkcją pierwszej klasy (lub zamknięcia pierwszej klasy): są to tylko dwa różne widoki tego samego rzecz.
Giorgio
4
@Giorgio Teoretycznie nie. W praktyce, jeśli język nie mieć co najmniej cukier składni dla niego, to jest różnica między fn x => x + 1i new Function<Integer, Integer>() { public Integer eval(Integer x) { return x + 1; }};czy ograniczenie się do kilku funkcji zakodowane i wąsko-użytecznych.
Doval,
Dlaczego nie fn x => x + 1można być cukrem syntaktycznym new Function<Integer, Integer>() { public Integer eval(Integer x) { return x + 1; }}? Ok, może masz na myśli, że ktoś utknął w języku bez tego rodzaju cukru syntaktycznego.
Giorgio
@Giorgio Musisz zapytać Oracle. (To prawda, że ​​w końcu zmienia się to w Javie 8). Zauważ, że potrzebujesz więcej cukru składniowego niż tylko to, aby być naprawdę przydatnym - chcesz również móc przekazywać wcześniej zadeklarowane metody według nazwy. Przydałoby się również curry. W pewnym momencie musisz się zastanowić, czy warto mieć te wszystkie hacki zamiast traktować funkcje jako pierwszych obywateli. Ale teraz jestem poza tematem.
Doval,
0

Jeśli stan posiadania sprawia, że ​​dana klasa jest bardziej użyteczna, funkcjonalna… wielokrotnego użytku, to nie. Z jakiegoś powodu przychodzi mi na myśl wzorzec projektowania poleceń ; z tego powodu z pewnością nie jest chęć utopienia Roberta Harveya.

radarbob
źródło
0

Zdefiniuj „często”. Jak często tworzysz obiekt? Dużo - prawdopodobnie nie?

Prawdopodobnie w twoim systemie są większe problemy. Przejście statyczne będzie wyraźniej informować programistę, że wewnętrzny stan obiektu się nie zmienia - świetnie.

Jeśli jednak pomieszany jest stan i musisz zacząć nadawać innym statyczny charakter, aby pierwsza stała się statyczna, musisz zapytać, które części muszą być statyczne, a które nie.

Tak, to zapach kodu, tylko niewielki.

użytkownik1775944
źródło
0

Zrób to metodą statyczną i idź dalej. Nie ma nic złego w metodach statycznych. Gdy pojawi się wzór użycia, możesz martwić się o jego abstrakcję.

davidk01
źródło
0

Zapach jest taki, że jest to kod proceduralny zawinięty (minimalnie) w obiekty.

Musi istnieć powód, dla którego chcesz wykonać tę operację na tabeli danych, ale zamiast modelować tę operację z innymi powiązanymi operacjami, które mają wspólną semantykę (tj. Mają wspólne znaczenie), autor po prostu uruchomił nową procedurę w nowej klasa i nazwał to.

W funkcjonalnym języku tak właśnie robisz;) ale w paradygmacie OO chciałbyś modelować abstrakcję obejmującą tę operację. Następnie użyłbyś technik OO do opracowania tego modelu i zapewnienia zestawu powiązanych operacji, które zachowują pewną semantykę.

Zatem głównym zapachem jest to, że autor używa klas, prawdopodobnie dlatego, że wymaga tego kompilator, bez organizowania operacji w model koncepcyjny.

BTW uważaj za każdym razem, gdy zobaczysz modelowany program zamiast przestrzeni problemowej . To znak, że projekt może zyskać na przemyśleniu. Innymi słowy, uważaj na klasy, z których wszystkie to „xAdaptor” oraz „xHandler” i „xUtility” i zwykle są powiązane z anemicznym modelem domeny . Oznacza to, że ktoś po prostu łączy kod dla wygody, w przeciwieństwie do faktycznego modelowania pojęć, które chce wdrożyć.

Obrabować
źródło