cette façon d'appeler une fonction est-elle une mauvaise pratique?

10

J'ai le code suivant:

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

Je pense qu'avec cette façon j'élimine la responsabilité de savoir ce qui est LatLngdans une autre classe, par exemple.

Et vous n'avez pas besoin de préparer les données avant d'appeler la fonction.

Qu'est-ce que tu penses?

Cette approche a-t-elle un nom? Est-ce vraiment une mauvaise pratique?

Tlaloc-ES
la source
2
Je pense que vous avez juste affaire à l'encapsulation en utilisant la surcharge de méthode intégrée du langage. Ce n'est pas bon / mauvais. Juste un outil qui peut aider ou blesser votre code.
bitsoflogic
5
Si votre objectif est de vous cacher LatLngdes clients de cette Cameraclasse, alors vous ne voulez probablement pas l' moveCameraTo(LatLng)être public.
bitsoflogic
En plus des conseils donnés dans les réponses, cela semble être un bon endroit pour mentionner YAGNI. Tu n'en auras pas besoin. Ne définissez pas de méthodes API avant qu'il y ait un bon cas d'utilisation parce que ... vous n'en aurez pas besoin.
Patrick Hughes
Rien de mal avec moveCameraToLocationet moveCameraTo/ moveCameraToCoords. Je ne voudrais certainement pas passer Location / lat / long tous sous les mêmes noms.
intérieur du

Réponses:

9

Vous utilisez votre fonctionnalité de surcharge de méthode de langues pour offrir à l'appelant d'autres moyens de résoudre la dépendance des méthodes sur les informations de position. Vous déléguez ensuite à une autre méthode pour résoudre le travail restant de mise à jour de la caméra.

L'odeur de code ici serait si vous continuez simplement à étendre la chaîne de méthodes appelant des méthodes. La méthode de prise de position appelle la méthode de prise double qui appelle la méthode de prise latLng qui appelle finalement quelque chose qui sait comment mettre à jour la caméra.

Les longues chaînes ne sont aussi solides que leur maillon le plus faible. Chaque extension de la chaîne augmente l'empreinte du code qui doit fonctionner ou cette chose se casse.

C'est une conception bien meilleure si chaque méthode emprunte le chemin le plus court possible pour résoudre le problème qui lui a été présenté. Cela ne signifie pas que chacun doit savoir comment mettre à jour la caméra. Chacun doit traduire son type de paramètre de position en un type uniforme qui peut être transmis à quelque chose qui sait comment mettre à jour la caméra lorsque ce type est présenté.

Faites-le de cette façon et vous pouvez en supprimer un sans casser la moitié de tout le reste.

Considérer:

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

Cela rend le problème de latitude et de longitude LatLng. Le coût est qu'il diffuse les connaissances LatLngautour. Cela peut sembler cher, mais d'après mon expérience, c'est une alternative préférable à l'obsession primitive, ce qui vous empêche de créer un objet paramètre .

Si Locationest refacturable mais LatLngne l'est pas, envisagez de résoudre ce problème en ajoutant une usine à Location:

moveCameraTo( location.ToLatLng() );

Cela évite également joliment l'obsession primitive.

candied_orange
la source
1
Cela ne me semble pas si mal - c'est vraiment juste une commodité pour le consommateur. Si cela était enchaîné 10 fois ou si le consommateur n'avait pas vraiment besoin de toutes ces options, j'appellerais cela une odeur de code.
mcknz
1
Mais quelle est l'alternative de mettre toute la logique pour transformer les doubles en LagLng ou Location en LagLng dans chaque fonction?
Tlaloc-ES
@ Tlaloc-ES mieux?
candied_orange
@ Tlaloc-ES La "transformation" ne doit se produire qu'une seule fois lorsque les primitives sont chargées dans la logique du domaine. Cela se produirait lorsque vous désérialiser le DTO. Ensuite, toute la logique de votre domaine arrive à circuler LatLngou des Locationobjets. Vous pouvez montrer la relation entre les deux avec New LatLng(Location)ou Location.toLatLng()si vous devez passer de l'un à l'autre.
bitsoflogic
1
@ Tlaloc-ES Lecture connexe: FlagArgument par Martin Fowler. Lisez la section "Implémentation enchevêtrée".
Marc.2377
8

Il n'y a rien de particulièrement mal avec votre solution.

Mais ma préférence personnelle serait que ces méthodes ne soient pas si utiles. Et compliquez simplement l'interface de tout objet dont ils font partie.

Le void moveCameraTo(double latitude, double longitude)ne simplifie pas vraiment le code, car je ne vois aucun problème appelant simplement moveCameraTo(new LatLng(latitude, longitude));à sa place. Cette méthode sent aussi l'obsession primitive.

Le void moveCameraTo(Location location)pourrait être mieux résolu en prouvant la Location.ToLatLng()méthode et en appelant moveCameraTo(location.ToLatLng()).

si c'était C # et si de telles méthodes étaient vraiment nécessaires, je les préférerais comme méthodes d'extension au lieu de méthodes d'instance. L'utilisation de méthodes d'extension deviendrait vraiment évidente si vous essayiez l'abstrait et testiez unitaire cette instance. Comme il serait beaucoup plus facile de simuler une seule méthode au lieu de plusieurs surcharges avec des conversions simples.

Je pense qu'avec cette façon, j'élimine la responsabilité de savoir ce qu'est un LatLng dans une autre classe, par exemple.

Je ne vois aucune raison pour laquelle ce serait un problème. Tant que votre code référence la classe qui contient void moveCameraTo(LatLng latLng), cela dépend toujours indirectement LatLng. Même si cette classe n'est jamais directement instanciée.

Et vous n'avez pas besoin de préparer les données avant d'appeler la fonction.

Je ne comprends pas ce que tu veux dire. Si cela signifie créer une nouvelle instance ou transformer des classes les unes des autres, je n'y vois aucun problème.

En y réfléchissant, je pense que ce que je dis est également pris en charge par la conception API de .NET lui-même. Historiquement, de nombreuses classes .NET ont suivi votre approche consistant à avoir de nombreuses surcharges avec différents paramètres et de simples conversions à l'intérieur. Mais c'était avant que les méthodes d'extension n'existent. Les classes .NET plus modernes sont plus légères dans leurs propres API et s'il existe des méthodes avec des surcharges de paramètres, elles sont fournies en tant que méthodes d'extension. Un exemple plus ancien est NLog ILogger qui a des dizaines de surcharges pour l'écriture dans le journal. Comparez cela aux nouveaux Microsoft.Extensions.Logging.ILogger qui ont un total de 3 méthodes (et seulement 1 si vous comptez vous connecter). Mais il existe de nombreux assistants et diverses paramétrisations comme méthodes d'extension .

Je pense que cette réponse montre que certaines langues auraient des outils pour rendre le design comme celui-ci plus agréable. Je ne connais pas beaucoup Java, donc je ne sais pas s'il y aurait un équivalent. Mais même l'utilisation de méthodes statiques simples peut être une option.

Euphorique
la source
Je ne sais pas pourquoi, mais je me retrouve enraciné pour cette réponse malgré une compétition. Je pense que vous avez juste réussi à faire avancer le point. Ma seule rétroaction serait de me rappeler que tous ceux qui traitent de ce problème ne sont pas sur .NET. +1
candied_orange
@candied_orange À droite. Maintenant que je regarde mieux la question d'OP, cela ressemble plus à Java qu'à C #.
Euphoric
Je pense qu'il n'y a vraiment pas de problème d'utilisation moveCameraTo(new LatLng(latitude, longitude));dans n'importe quel endroit du projet, mais je pense que c'est plus clair d'utiliser directement le moveCametaTo (latLng), c'est comme un fichier en java que vous pouvez passer comme une chaîne, ou comme une classe Path
Tlaloc-ES
1

Je ne sais pas quel est le nom correct pour cela, s'il en a un, mais c'est une bonne pratique. Vous exposez plusieurs surcharges, permettant à la classe appelante de déterminer le jeu de paramètres qu'elle souhaite utiliser. Comme vous le dites, une autre classe peut ne pas savoir ce qu'est un LatLngobjet, mais peut le savoir Location.

Avoir une méthode appeler l'autre est également essentiel, car vous ne voulez pas de code dupliqué entre ces méthodes. Comme vous l'avez fait, choisissez une méthode pour être celle qui fait le travail et demandez aux autres méthodes de l'appeler (directement ou indirectement)

mmathis
la source
1

Si cela vous dérange d'utiliser des méthodes, c'est juste une fonctionnalité de surcharge de méthode, c'est bien.

Mais cela n'élimine pas la responsabilité de savoir ce qu'est un LatLng . Parce que vous initialisez LatLng latLng = new LatLng(latitude, longitude). C'est une dépendance totale à LatLng. (Pour comprendre pourquoi l'initialisation est un problème de dépendance, vous pouvez vérifier l' injection de dépendance ) La création d'une méthode surchargée aide simplement les clients qui ne s'en soucient pas LatLng. Si vous voulez dire cela, c'est aussi bien mais je ne pense pas que ce soit une approche. Il s'agit simplement de nombreuses méthodes de service pour les clients.

Il existe donc deux options pour concevoir votre architecture:

  1. Créez de nombreuses méthodes surchargées et fournissez-les au client.
  2. Créez quelques méthodes surchargées qui nécessitent des paramètres comme interface ou classe concrète.

Je m'enfuis le plus possible en créant des méthodes qui nécessitent des types primitifs comme paramètres (Option 1). Parce que si votre entreprise change plusieurs fois et que vous devez lire les paramètres de méthode, il est très difficile de modifier et d'implémenter toutes les fonctions d'appel.

Au lieu de cela, utilisez des interfaces (injection de dépendance). Si vous pensez que c'est coûteux et prend plus de temps, utilisez des classes et fournissez leurs méthodes d'extension de mappeur (Option 2).

Engineert
la source