czy ten sposób nazywania funkcji jest złą praktyką?

10

Mam następujący kod:

public void moveCameraTo(Location location){
    moveCameraTo(location.getLatitude(), location.getLongitude());
}

public void moveCameraTo(double latitude, double longitude){
    LatLng latLng = new LatLng(latitude, longitude);
    moveCameraTo(latLng);
}

public void moveCameraTo(LatLng latLng){
    GoogleMap googleMap =  getGoogleMap();
    cameraUpdate = CameraUpdateFactory.newLatLngZoom(latLng, INITIAL_MAP_ZOOM_LEVEL);
    googleMap.moveCamera(cameraUpdate);
}

Myślę, że w ten sposób eliminuję na przykład odpowiedzialność za to, co jest LatLngw innej klasie.

I nie trzeba przygotowywać danych przed wywołaniem funkcji.

Co myślisz?

Czy to podejście ma nazwę? Czy to naprawdę zła praktyka?

Tlaloc-ES
źródło
2
Myślę, że masz do czynienia z enkapsulacją za pomocą wbudowanej w język metody Overloading. To nie jest dobre / złe. Tylko narzędzie, które może pomóc lub zranić Twój kod.
bitsoflogic
5
Jeśli Twoim celem jest ukrycie się LatLngprzed klientami tej Cameraklasy, prawdopodobnie nie chcesz moveCameraTo(LatLng)być public.
bitsoflogic
Oprócz rad zawartych w odpowiedziach jest to dobre miejsce, aby wspomnieć o YAGNI. Nie będziesz go potrzebował. Nie definiuj metod API, zanim nie będzie dobrego przypadku użycia, ponieważ ... Nie będziesz go potrzebował.
Patrick Hughes,
Nic złego w / moveCameraToLocationi . Zdecydowanie nie chciałbym przekazywać lokalizacji / lat / long o tych samych nazwach. moveCameraTomoveCameraToCoords
środku

Odpowiedzi:

9

Używasz funkcji przeciążania metod w swoich językach, aby zaoferować dzwoniącemu alternatywne sposoby rozwiązania zależności metod od informacji o położeniu. Następnie delegujesz na inną metodę, aby rozwiązać pozostałe prace związane z aktualizacją kamery.

Zapach kodu będzie tutaj, jeśli będziesz po prostu rozszerzać łańcuch metod wywołujących metody. Metoda pobierania lokalizacji wywołuje metodę podwójnego pobierania, która wywołuje metodę pobierania LatLng, która w końcu wywołuje coś, co wie, jak zaktualizować kamerę.

Długie łańcuchy są tak silne, jak ich najsłabsze ogniwo. Każde przedłużenie łańcucha zwiększa ślad kodu, który musi działać lub coś się psuje.

Jest to znacznie lepszy projekt, jeśli każda metoda podąża możliwie najkrótszą drogą do rozwiązania przedstawionego mu problemu. To nie znaczy, że każdy musi wiedzieć, jak zaktualizować kamerę. Każdy powinien przetłumaczyć swój typ parametru pozycji na jeden jednolity typ, który można przekazać do czegoś, co wie, jak zaktualizować kamerę, gdy zostanie przedstawiony ten jeden typ.

Zrób to w ten sposób, a możesz usunąć jeden bez zerwania połowy całej reszty.

Rozważać:

public void moveCameraTo(Location location){
    moveCameraTo( new LatLng(location) );
}

To sprawia, że ​​mamy do czynienia z LatLngproblemem szerokości i długości geograficznej . Kosztem jest to, że rozpowszechnia wiedzę o LatLngokolicy. To może wydawać się drogie, ale z mojego doświadczenia wynika, że ​​jest to lepsza alternatywa dla prymitywnej obsesji, dzięki której unikanie ustanowienia obiektu parametru sprawia, że ​​utkniesz.

Jeśli Locationmożna go refaktoryzować, ale LatLngnie jest, rozważ rozwiązanie tego problemu poprzez dodanie fabryki do Location:

moveCameraTo( location.ToLatLng() );

To także ładnie unika prymitywnej obsesji.

candied_orange
źródło
1
Nie wydaje mi się to takie złe - to tak naprawdę tylko wygoda dla konsumenta. Gdyby to było powiązane 10 razy lub jeśli konsument tak naprawdę nie potrzebuje wszystkich tych opcji, nazwałbym to zapachem kodu.
mcknz
1
Ale jaką alternatywą jest umieszczenie całej logiki podwójnej transformacji w LagLng lub Location w LagLng w każdej funkcji?
Tlaloc-ES,
@ Tlaloc-ES lepiej?
candied_orange
@ Tlaloc-ES „Transformacja” musi nastąpić tylko raz, gdy operacje podstawowe są ładowane do logiki domeny. Wystąpiłoby to, gdy deserializujesz DTO. Następnie cała logika domeny będzie przekazywana dookoła LatLnglub Locationobiektów. Możesz pokazać związek między nimi za pomocą New LatLng(Location)lub, Location.toLatLng()jeśli chcesz przejść od jednego do drugiego.
bitsoflogic
1
@ Tlaloc-ES Powiązane teksty : FlagArgument autorstwa Martina Fowlera. Przeczytaj sekcję „Tangled Implementation”.
Marc.2377
8

Nie ma nic szczególnie złego w twoim rozwiązaniu.

Ale osobiście wolę, aby te metody nie były tak przydatne. I po prostu skomplikuj interfejs dowolnego obiektu, z którego są wyłączone.

To void moveCameraTo(double latitude, double longitude)tak naprawdę nie upraszcza kodu, ponieważ nie widzę problemu, aby po prostu zadzwonić moveCameraTo(new LatLng(latitude, longitude));w to miejsce. Ta metoda pachnie także prymitywną obsesją.

void moveCameraTo(Location location)Mogłoby być lepiej rozwiązane poprzez wykazanie Location.ToLatLng()sposobu i powołanie moveCameraTo(location.ToLatLng()).

gdyby to był C # i gdyby takie metody były naprawdę konieczne, wolałbym je jako metody rozszerzenia zamiast metod instancji. Zastosowanie metod rozszerzenia stałoby się naprawdę oczywiste, gdybyś spróbował abstrakcji i przetestował to urządzenie. Ponieważ o wiele łatwiej byłoby po prostu sfałszować jedną metodę zamiast wielu przeciążeń za pomocą prostych konwersji.

Myślę, że w ten sposób eliminuję na przykład odpowiedzialność za znajomość języka LatLng w innej klasie.

Nie widzę powodu, dla którego miałby to stanowić problem. Tak długo, jak twój kod odwołuje się do klasy, która zawiera void moveCameraTo(LatLng latLng), nadal zależy pośrednio LatLng. Nawet jeśli ta klasa nigdy nie jest bezpośrednio tworzona.

I nie trzeba przygotowywać danych przed wywołaniem funkcji.

Nie rozumiem o co ci chodzi. Jeśli oznacza to utworzenie nowej instancji lub przekształcenie klas z jednej w drugą, nie widzę z tym problemu.

Myśląc o tym, czuję, że to, co mówię, jest również wspierane przez projekt API samego .NET. Historycznie wiele klas .NET podążało za twoim podejściem, mając wiele przeciążeń z różnymi parametrami i prostymi konwersjami wewnątrz. Ale to było zanim istniały metody rozszerzenia. Bardziej nowoczesne klasy .NET są bardziej lekkie we własnych interfejsach API, a jeśli istnieją metody z przeciążeniem parametrów, są one dostarczane jako metody rozszerzenia. Starszym przykładem jest NLog ILogger, który ma dziesiątki przeciążeń związanych z pisaniem do dziennika. Porównaj to z nowszym Microsoft.Extensions.Logging.ILogger, który ma w sumie 3 metody (i tylko 1, jeśli policzysz rejestrowanie). Ale istnieje wiele pomocników i różnych parametryzacji jako metod rozszerzenia .

Myślę, że ta odpowiedź pokazuje, że niektóre języki miałyby narzędzia, dzięki którym projektowanie takie byłoby ładniejsze. Nie znam dużo Javy, więc nie jestem pewien, czy będzie jakiś odpowiednik. Ale nawet przy użyciu zwykłych metod statycznych może być opcją.

Euforyk
źródło
Nie jestem pewien, dlaczego, ale uważam, że kibicuję tej odpowiedzi, mimo że mam konkurencyjną. Myślę, że właśnie udało ci się poprawić punkt. Chciałbym jedynie pamiętać, że nie wszyscy zajmujący się tym problemem są w .NET. +1
candied_orange
@candied_orange Right. Teraz, gdy patrzę lepiej na pytanie OP, które bardziej przypomina Javę niż C #.
Euphoric
Myślę, że tak naprawdę nie ma problemu w użyciu moveCameraTo(new LatLng(latitude, longitude));w żadnym miejscu projektu, ale myślę, że jest to bardziej zrozumiałe, użycie bezpośrednio moveCametaTo (latLng), jest jak plik w Javie, który można przekazać ścieżką jak ciąg znaków, lub jak klasa ścieżki
Tlaloc-ES,
1

Nie jestem pewien, jak się nazywa, jeśli tak, ale to dobra praktyka. Wystawiasz wiele przeciążeń, pozwalając klasie wywołującej określić, którego zestawu parametrów chce użyć. Jak mówisz, inna klasa może nie wiedzieć, czym jest LatLngobiekt, ale może go znać Location.

Kluczem jest także wywołanie jednej metody, ponieważ nie chcesz duplikować kodu w tych metodach. Po zakończeniu wybierz jedną metodę, która będzie działać, i niech inne metody ją wywołają (bezpośrednio lub pośrednio)

mmathis
źródło
1

Jeśli masz nic przeciwko użyciu metod, jest to po prostu funkcja przeciążania metod.

Ale nie eliminuje się odpowiedzialności za znajomość języka LatLng . Ponieważ inicjujesz LatLng latLng = new LatLng(latitude, longitude). Jest to całkowicie zależność od LatLng. (Aby zrozumieć, dlaczego inicjowanie jest problemem zależności, możesz sprawdzić Wstrzykiwanie zależności ). Utworzenie przeciążonej metody pomaga tylko klientom, którym to nie zależy LatLng. Jeśli masz na myśli to, jest to również dobre, ale nie sądzę, że jest to podejście. To tylko wiele metod obsługi klientów.

Istnieją więc dwie opcje zaprojektowania architektury:

  1. Utwórz wiele przeciążonych metod i przekaż je klientowi.
  2. Utwórz kilka przeciążonych metod, które wymagają parametrów jako interfejsu lub konkretnej klasy.

Uciekam od tworzenia metod, które wymagają prymitywnych typów jako parametrów (opcja 1). Ponieważ jeśli Twoja firma zmienia się wiele razy i musisz odtworzyć parametry metody, naprawdę trudno jest zmienić i wdrożyć wszystkie funkcje dzwoniącego.

Zamiast tego użyj interfejsów (wstrzykiwanie zależności). Jeśli uważasz, że jest to kosztowne i zajmuje więcej czasu, skorzystaj z klas i podaj ich metody rozszerzenia mapera (Opcja 2).

Engineert
źródło