2017-09-26 3 views
0

J'ai une grande méthode qui ressemble à ceciCommutateurs imbriqués ou fonctions multiples, quel serait un meilleur design?

List<Hotel> findAvailHotels(Provider provider, Method method, List<String> codes) { 
    switch (provider) { 
     case PROVIDER_1: 
      //TODO Do common things to provider 1 
      switch (method) { 
       case HOTEL_CODE: 
        break; 
       case DESTINATION_CODE: 
        break; 
       case GEO: 
        break; 
      } 
      break; 
     case PROVIDER_2: 
      switch (method) { 
       case HOTEL_CODE: 
        break; 
       case DESTINATION_CODE: 
        break; 
       case GEO: 
        break; 
      } 
      break; 
    } 

Donc, chaque fois que je dois ajouter un fournisseur je besoin d'ajouter un cas à ce fournisseur, puis répétez le commutateur method pour ce nouveau fournisseur.

je une suggestion d'un collègue qui devrait être divisé en méthodes pour chaque method donc par exemple au lieu de ce qui précède, il sera

List<Hotel> findAvailHotelsByHotelCode(Provider provider, List<String> codes) { 
    switch (provider) { 
     case PROVIDER_1: 
      //TODO Do common things to provider 1 
      break; 
     case PROVIDER_2: 
      break; 
    } 

List<Hotel> findAvailHotelsByDestinationCode(Provider provider, List<String> codes) { 
    switch (provider) { 
     case PROVIDER_1: 
      //TODO Do common things to provider 1 
      break; 
     case PROVIDER_2: 
      break; 
    } 

List<Hotel> findAvailHotelsByGeo(Provider provider, List<String> codes) { 
    switch (provider) { 
     case PROVIDER_1: 
      //TODO Do common things to provider 1 
      break; 
     case PROVIDER_2: 
      break; 
    } 

pensées personnelles: Peut-être diviser en plusieurs méthodes le rend plus propre, mais si j'ai besoin de faire des choses communes à PROVIDER_1 (malgré le method) alors cette chose commune devra être répétée/dupliquée dans chaque méthode (comme indiqué par les //TODO s dans le code ci-dessus) qui signifie un peu plus de lignes de code, mais c'est un peu hors de propos peut-être.

J'aimerais avoir quelques réflexions à ce sujet, que considéreriez-vous comme plus lisibles et plus propres? De meilleures alternatives?


modifier: Pour donner plus de contexte, je travaille avec les fournisseurs hôteliers .. la plupart des fournisseurs ont 3 méthodes de recherche commun (hotel_code, destination_code, géo) .. à partir de l'extérieur de cette méthode que je peux faire une hotel_code recherche pour tous les fournisseurs (en bouclant sur le fournisseur enum et en appelant la méthode pour chaque fournisseur avec hotel_code enum param) ou je peux le faire à un fournisseur spécifique.

+0

Pas sympa, mais faisable sans les instructions switch: Reflection. D'un autre côté, il semble que vous pourriez résoudre ce problème en utilisant l'héritage. Recueillir des informations communes dans la classe de base et mettre la partie de modification dans les classes enfant – ParkerHalo

+1

@ParkerHalo l'utilisation de * réflexion * devrait être le dernier recours pour faire quelque chose en Java! –

+0

@TimothyTruckle C'est pourquoi j'ai écrit _ "Pas gentil" _... C'est aussi pourquoi je n'ai pas fait de réponse et suggéré héritage/polymorphisme – ParkerHalo

Répondre

2

Votre question est encore un peu trop abstraite pour suggérer une "meilleure" solution, mais Timothy a raison jusqu'à présent - dans les deux cas, vous pouvez utiliser le polimorphisme. Je suggère le modèle de stratégie parce que vous définissez la structure large en utilisant une interface et créez une classe dédiée pour chaque algorithme (fournisseur dans votre cas).

Cela a au moins deux avantages:

  1. Vous avez un outil facile à surveiller Liste des algorithmes sous forme de classes.
  2. Vous pouvez remplacer le commutateur externe par une boucle dans vos objets de stratégie.

Hmm - puisque vous l'avez demandé - voici quelques exemples de code (un peu grand mais ...)

import java.util.ArrayList; 
import java.util.List; 

public class HotelStuff { 

    private static class Hotel{/* does whatever ...*/} 

    private enum SearchMethod{ 
    HOTELCODE, 
    DESTINATIONCODE, 
    GEOCODE 
    } 

    private interface Providable{ 
    List<Hotel> findAvailHotels(SearchMethod method, List<String> codes); 
    } 

    private static class Provider1 implements Providable{ 
    @Override 
    public List<Hotel> findAvailHotels(SearchMethod method, List<String> codes) { 
     // TODO create the list ... 
     return null; 
    } 
    } 

    public static void main(String[] args) { 
    // TODO Auto-generated method stub 
    List<Providable> providers = new ArrayList<Providable>(); 
    providers.add(new Provider1()); 
    // providers.add(new Provider2 .. and so on  
    List<String> codes = Arrays.asList("123","456"); 
    SearchMethod method = SearchMethod.GEOCODE; 
    List<Hotel> availableHotels = findAvailHotels(providers, method, codes); 
    } 

    public static List<Hotel> findAvailHotels(List<Providable> providers, SearchMethod method, List<String> codes) { 
    List<Hotel> result = new ArrayList<Hotel>(); 
    List<Hotel> partResult; 
    for(Providable provider: providers) { 
     partResult = provider.findAvailHotels(method, codes); 
     result.addAll(partResult); 
    } 
    return result; 
    } 

} 

Bien sûr, vous devez implémenter les classes dans des fichiers séparés - je les ai juste mis dans un fichier pour le raccourcir.

+0

J'ai édité le code ci-dessus et ajouté plus de contexte pour le rendre plus clair – prettyvoid

+0

Eh bien, alors. Puisque vos méthodes "findAvailHotels ..." ont déjà la même signature, vous pouvez unir les trois à une seule méthode avec un paramètre de méthode et l'utiliser dans votre interface. – ospf

+0

Seriez-vous aimable de donner un exemple simple chaque fois que vous avez le temps? Parce que ça va me faire comprendre plus clairement. Merci – prettyvoid

1

À moins que votre déclaration switch est en vous devriez mieux utiliser usine polymorphisme.

0

Vous devriez regarder dans le modèle de visiteur et la double expédition.

la bande des quatre définit le visiteur:

représentent une opération à effectuer sur des éléments d'une structure d'objet. Visitor vous permet de définir une nouvelle opération sans changer les classes des éléments sur lesquels elle opère.

Dans votre cas fournisseur est l'objet et la méthode est l'opération. La répartition double est utile dans les situations où le choix du calcul dépend des types d'exécution de ses arguments. Dans votre cas: vous voulez faire quelque chose basé sur le type de Fournisseur et Méthode.