Ist diese Art, eine Funktion aufzurufen, eine schlechte Praxis?

10

Ich habe folgenden Code:

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);
}

Ich denke, dass ich auf diese Weise die Verantwortung eliminiere, zu wissen, was LatLngzum Beispiel in einer anderen Klasse ist.

Und Sie müssen die Daten nicht vorbereiten, bevor Sie die Funktion aufrufen.

Was denken Sie?

Hat dieser Ansatz einen Namen? Ist es wirklich eine schlechte Praxis?

Tlaloc-ES
quelle
2
Ich denke, Sie beschäftigen sich nur mit der Kapselung mithilfe der in der Sprache integrierten Methodenüberladung. Es ist nicht gut / schlecht. Nur ein Tool, das Ihrem Code helfen oder schaden kann.
Bitsoflogic
5
Wenn es Ihr Ziel ist, sich LatLngvor den Klienten dieser CameraKlasse zu verstecken , dann möchten Sie wahrscheinlich nicht moveCameraTo(LatLng)sein public.
Bitsoflogic
Zusätzlich zu den Ratschlägen in den Antworten scheint dies ein guter Ort zu sein, um YAGNI zu erwähnen. Du wirst es nicht brauchen. Definieren Sie keine API-Methoden, bevor es einen guten Anwendungsfall gibt, weil ... Sie ihn nicht brauchen werden.
Patrick Hughes
Nichts falsch mit moveCameraToLocationund moveCameraTo/ moveCameraToCoords. Ich würde definitiv nicht Location / lat / long alle mit den gleichen Namen übergeben wollen.
Insidesin

Antworten:

9

Sie verwenden die Methodenüberladungsfunktion Ihrer Sprache, um dem Aufrufer alternative Möglichkeiten zum Auflösen der Methodenabhängigkeit von Positionsinformationen anzubieten. Sie delegieren dann an eine andere Methode, um die verbleibende Arbeit beim Aktualisieren der Kamera zu lösen.

Der Code-Geruch hier wäre, wenn Sie nur die Kette von Methoden erweitern, die Methoden aufrufen. Die Location-Taking-Methode ruft die Double-Taking-Methode auf, die die latLng-Taking-Methode aufruft, die schließlich etwas aufruft, das weiß, wie die Kamera aktualisiert wird.

Lange Ketten sind nur so stark wie ihr schwächstes Glied. Jede Erweiterung der Kette erhöht den Platzbedarf von Code, der funktionieren muss, oder dieses Ding bricht.

Es ist ein weitaus besseres Design, wenn jede Methode den kürzestmöglichen Weg zur Lösung des ihr vorgelegten Problems einschlägt. Das bedeutet nicht, dass jeder wissen muss, wie die Kamera aktualisiert wird. Jeder sollte seinen Positionsparametertyp in einen einheitlichen Typ übersetzen, der an etwas übergeben werden kann, das weiß, wie die Kamera aktualisiert wird, wenn dieser eine Typ angezeigt wird.

Wenn Sie dies so tun, können Sie eine entfernen, ohne die Hälfte von allem anderen zu beschädigen.

Erwägen:

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

Dies macht den Umgang mit Längen- und Breitengraden LatLngproblematisch. Die Kosten sind, dass es Wissen verbreitet LatLng. Das mag teuer erscheinen, aber meiner Erfahrung nach ist es eine bevorzugte Alternative zur primitiven Besessenheit. Wenn Sie vermeiden, ein Parameterobjekt einzurichten , bleiben Sie dabei.

Wenn Locationrefactorable ist aber LatLngnicht, sollten Sie diese Lösung durch eine Fabrik Zugabe Location:

moveCameraTo( location.ToLatLng() );

Dies vermeidet auch gut primitive Besessenheit.

candied_orange
quelle
1
Scheint mir nicht so schlimm - es ist wirklich nur eine Annehmlichkeit für den Verbraucher. Wenn dies 10 Mal verkettet wäre oder wenn der Verbraucher nicht wirklich alle diese Optionen benötigt, würde ich es einen Code-Geruch nennen.
mcknz
1
Aber was ist die Alternative, alle Logik für Transformationsdoppel in LagLng oder Ort in LagLng in jeder Funktion zu setzen?
Tlaloc-ES
@ Tlaloc-ES besser?
candied_orange
@ Tlaloc-ES Die "Transformation" muss nur einmal erfolgen, wenn die Grundelemente in die Domänenlogik geladen werden. Es würde auftreten, wenn Sie das DTO deserialisieren. Dann kann Ihre gesamte Domänenlogik LatLngoder LocationObjekte weitergegeben werden. Sie können die Beziehung zwischen den beiden mit New LatLng(Location)oder zeigen, Location.toLatLng()wenn Sie von einem zum anderen gehen müssen.
Bitsoflogic
1
@ Tlaloc-ES Verwandte Lesung: FlagArgument von Martin Fowler. Lesen Sie den Abschnitt "Verwickelte Implementierung".
März 2377
8

An Ihrer Lösung ist nichts besonders auszusetzen.

Aber meine persönliche Präferenz wäre, dass diese Methoden nicht so nützlich sind. Und komplizieren Sie einfach die Schnittstelle des Objekts, von dem sie getrennt sind.

Das void moveCameraTo(double latitude, double longitude)vereinfacht den Code nicht wirklich, da ich kein Problem sehe, einfach moveCameraTo(new LatLng(latitude, longitude));an seiner Stelle anzurufen. Diese Methode riecht auch nach primitiver Besessenheit.

Das void moveCameraTo(Location location)könnte besser gelöst werden, indem Location.ToLatLng()Methode und Aufruf bewiesen werden moveCameraTo(location.ToLatLng()).

Wenn dies C # wäre und solche Methoden wirklich notwendig wären, würde ich sie als Erweiterungsmethoden anstelle von Instanzmethoden bevorzugen. Die Verwendung von Erweiterungsmethoden wird sehr offensichtlich, wenn Sie versuchen, diese Instanz zu abstrahieren und einem Unit-Test zu unterziehen. Da es viel einfacher wäre, nur eine einzelne Methode zu fälschen, anstatt mehrere Überladungen mit einfachen Konvertierungen.

Ich denke, dass ich auf diese Weise die Verantwortung eliminiere, zum Beispiel zu wissen, was ein LatLng in einer anderen Klasse ist.

Ich sehe keinen Grund, warum dies ein Problem sein würde. Solange Ihr Code auf eine Klasse verweist, die diese enthält void moveCameraTo(LatLng latLng), hängt dies indirekt davon ab LatLng. Auch wenn diese Klasse niemals direkt instanziiert wird.

Und Sie müssen die Daten nicht vorbereiten, bevor Sie die Funktion aufrufen.

Ich verstehe nicht, was du meinst. Wenn es bedeutet, eine neue Instanz zu erstellen oder Klassen von einer in eine andere umzuwandeln, sehe ich kein Problem damit.

Wenn ich darüber nachdenke, habe ich das Gefühl, dass das, was ich sage, auch vom API-Design von .NET selbst unterstützt wird. In der Vergangenheit folgten viele .NET-Klassen Ihrem Ansatz, viele Überladungen mit unterschiedlichen Parametern und einfache Konvertierungen im Inneren zu haben. Aber das war, bevor es Erweiterungsmethoden gab. Moderne .NET-Klassen sind in ihren eigenen APIs leichter und wenn es Methoden mit Parameterüberladungen gibt, werden sie als Erweiterungsmethoden bereitgestellt. Älteres Beispiel ist NLog ILogger mit Dutzenden von Überladungen zum Schreiben in das Protokoll. Vergleichen Sie dies mit dem neueren Microsoft.Extensions.Logging.ILogger mit insgesamt 3 Methoden (und nur 1, wenn Sie die Protokollierung selbst zählen). Es gibt jedoch viele Helfer und verschiedene Parametrisierungen als Erweiterungsmethoden .

Ich denke, diese Antwort zeigt, dass einige Sprachen Werkzeuge haben würden, um Design wie dieses schöner zu machen. Ich kenne nicht viel Java, daher bin ich mir nicht sicher, ob es ein Äquivalent geben würde. Aber auch die Verwendung einfacher statischer Methoden könnte eine Option sein.

Euphorisch
quelle
Ich weiß nicht warum, aber ich bin auf der Suche nach dieser Antwort, obwohl ich eine konkurrierende habe. Ich denke, Sie haben es gerade geschafft, den Punkt besser zu machen. Mein einziges Feedback wäre, mich daran zu erinnern, dass nicht jeder, der sich mit diesem Problem befasst, auf .NET ist. +1
candied_orange
@candied_orange Richtig. Jetzt, wo ich die Frage von OP besser betrachte, sieht das eher nach Java als nach C # aus.
Euphoric
Ich denke , dass wirklich kein Problem im Einsatz moveCameraTo(new LatLng(latitude, longitude));in jedem Ort des Projektes, aber ich denke , dass mehr klar Gebrauch ist direkt die moveCametaTo (latLng), wie eine Datei in Java ist , dass Sie Pfad wie String oder wie eine Path - Klasse passieren können
Tlaloc-ES
1

Ich bin mir nicht sicher, wie der richtige Name dafür lautet, wenn es einen gibt, aber es ist eine gute Praxis. Sie stellen mehrere Überladungen bereit, sodass die aufrufende Klasse bestimmen kann, welchen Parametersatz sie verwenden möchte. Wie Sie sagen, weiß eine andere Klasse möglicherweise nicht, was ein LatLngObjekt ist, aber möglicherweise, was es ist Location.

Es ist auch wichtig, dass eine Methode die andere aufruft, da Sie keinen doppelten Code für diese Methoden wünschen. Wählen Sie wie Sie eine Methode aus, die die Arbeit erledigt, und lassen Sie die anderen Methoden sie aufrufen (direkt oder indirekt).

mmathis
quelle
1

Wenn es Ihnen etwas ausmacht, Methoden zu verwenden, ist dies nur eine Funktion zum Überladen von Methoden.

Aber es ist nicht zu beseitigen , die Verantwortung zu wissen , was ein LatLng ist . Weil Sie initialisieren LatLng latLng = new LatLng(latitude, longitude). Dies ist eine völlige Abhängigkeit von LatLng. (Um zu verstehen, warum das Initialisieren ein Abhängigkeitsproblem ist, können Sie die Abhängigkeitsinjektion überprüfen. ) Das Erstellen einer überladenen Methode hilft nur Clients, die sich nicht darum kümmern LatLng. Wenn Sie das meinen, ist es auch gut, aber ich denke nicht, dass es ein Ansatz ist. Es sind nur viele Servicemethoden für Kunden.

Es gibt also zwei Möglichkeiten, Ihre Architektur zu entwerfen:

  1. Erstellen Sie viele überladene Methoden und stellen Sie sie dem Client zur Verfügung.
  2. Erstellen Sie einige überladene Methoden, die Parameter als Schnittstelle oder konkrete Klasse benötigen.

Ich laufe so weit wie möglich davon ab, Methoden zu erstellen, die primitive Typen als Parameter benötigen (Option 1). Denn wenn sich Ihr Unternehmen viele Male ändert und Sie Methodenparameter spielen müssen, ist es wirklich schwierig, alle Aufruferfunktionen zu ändern und zu implementieren.

Verwenden Sie stattdessen Schnittstellen (Dependency Injection). Wenn Sie der Meinung sind, dass dies kostenintensiv ist und mehr Zeit in Anspruch nimmt, verwenden Sie Klassen und geben Sie deren Mapper-Erweiterungsmethoden an (Option 2).

Engineert
quelle