Éviter instanceof en Java

Avoir une chaîne d'opérations "instanceof" est considéré comme une "odeur de code". La réponse standard est "utiliser le polymorphisme". Comment le ferais-je dans ce cas?

Il existe un certain nombre de sous-classes d'une classe de base; aucun d'entre eux n'est sous mon contrôle. Une situation analogue serait avec les classes Java Integer, Double, BigDecimal etc.

if (obj instanceof Integer) {NumberStuff.handle((Integer)obj);}
else if (obj instanceof BigDecimal) {BigDecimalStuff.handle((BigDecimal)obj);}
else if (obj instanceof Double) {DoubleStuff.handle((Double)obj);}

J'ai le contrôle sur NumberStuff et ainsi de suite.

Je ne veux pas utiliser beaucoup de lignes de code là où quelques lignes feraient l'affaire. (Parfois, je fais un HashMap mappant Integer.class à une instance de IntegerStuff, BigDecimal.class à une instance de BigDecimalStuff etc. Mais aujourd'hui, je veux quelque chose de plus simple.)

J'aimerais quelque chose d'aussi simple que ceci:

public static handle(Integer num) { ... }
public static handle(BigDecimal num) { ... }

Mais Java ne fonctionne tout simplement pas de cette façon.

Je voudrais utiliser des méthodes statiques lors du formatage. Les choses que je formate sont composites, où un Thing1 peut contenir un tableau Thing2s et un Thing2 peut contenir un tableau de Thing1s. J'ai eu un problème lorsque j'ai implémenté mes formateurs comme ceci:

class Thing1Formatter {
  private static Thing2Formatter thing2Formatter = new Thing2Formatter();
  public format(Thing thing) {
      thing2Formatter.format(thing.innerThing2);
  }
}
class Thing2Formatter {
  private static Thing1Formatter thing1Formatter = new Thing1Formatter();
  public format(Thing2 thing) {
      thing1Formatter.format(thing.innerThing1);
  }
}

Oui, je connais le HashMap et un peu plus de code peut aussi résoudre ce problème. Mais l '"instanceof" semble si lisible et maintenable par comparaison. Y a-t-il quelque chose de simple mais pas malodorant?

Note ajoutée le 5/10/2010:

Il s'avère que de nouvelles sous-classes seront probablement ajoutées à l'avenir, et mon code existant devra les gérer avec élégance. Le HashMap sur la classe ne fonctionnera pas dans ce cas car la classe ne sera pas trouvée. Une chaîne d'instructions if, commençant par la plus spécifique et se terminant par la plus générale, est probablement la meilleure après tout:

if (obj instanceof SubClass1) {
    // Handle all the methods and properties of SubClass1
} else if (obj instanceof SubClass2) {
    // Handle all the methods and properties of SubClass2
} else if (obj instanceof Interface3) {
    // Unknown class but it implements Interface3
    // so handle those methods and properties
} else if (obj instanceof Interface4) {
    // likewise.  May want to also handle case of
    // object that implements both interfaces.
} else {
    // New (unknown) subclass; do what I can with the base class
}

Vous pourriez être intéressé par cette entrée du blog Amazon de Steve Yegge: "quand le polymorphisme échoue" . Essentiellement, il traite de cas comme celui-ci, où le polymorphisme cause plus de problèmes qu'il n'en résout.

Le problème est que pour utiliser le polymorphisme, vous devez intégrer la logique du «traitement» à chaque classe de «commutation» - c'est-à-dire Integer, etc. dans ce cas. Ce n'est manifestement pas pratique. Parfois, ce n'est même pas logiquement le bon endroit pour mettre le code. Il recommande l'approche «instanceof» comme étant le moindre de plusieurs maux.

Comme dans tous les cas où vous êtes obligé d'écrire du code malodorant, gardez-le boutonné dans une méthode (ou au plus une classe) afin que l'odeur ne s'échappe pas.

Comme souligné dans les commentaires, le modèle de visiteur serait un bon choix. Mais sans contrôle direct sur la cible / l'accepteur / le visiteur, vous ne pouvez pas implémenter ce modèle. Voici une façon dont le modèle de visiteur pourrait encore être utilisé ici même si vous n'avez aucun contrôle direct sur les sous-classes en utilisant des wrappers (en prenant Integer comme exemple):

public class IntegerWrapper {
    private Integer integer;
    public IntegerWrapper(Integer anInteger){
        integer = anInteger;
    }
    //Access the integer directly such as
    public Integer getInteger() { return integer; }
    //or method passthrough...
    public int intValue() { return integer.intValue(); }
    //then implement your visitor:
    public void accept(NumericVisitor visitor) {
        visitor.visit(this);
    }
}

Bien sûr, envelopper une classe finale peut être considéré comme une odeur en soi, mais peut-être convient-il bien à vos sous-classes. Personnellement, je ne pense pas que instanceofce soit une mauvaise odeur ici, surtout si elle se limite à une méthode et que je l'emploierais volontiers (probablement sur ma propre suggestion ci-dessus). Comme vous le dites, c'est assez lisible, dactylographié et maintenable. Comme toujours, restez simple.

Au lieu d'un énorme if, vous pouvez placer les instances que vous gérez dans une carte (clé: classe, valeur: gestionnaire).

Si la recherche par clé retourne null, appelez une méthode de gestionnaire spéciale qui tente de trouver un gestionnaire correspondant (par exemple en appelant isInstance()chaque clé de la carte).

Lorsqu'un gestionnaire est trouvé, enregistrez-le sous la nouvelle clé.

Cela rend le cas général rapide et simple et vous permet de gérer l'héritage.

Vous pouvez utiliser la réflexion:

public final class Handler {
  public static void handle(Object o) {
    try {
      Method handler = Handler.class.getMethod("handle", o.getClass());
      handler.invoke(null, o);
    } catch (Exception e) {
      throw new RuntimeException(e);
    }
  }
  public static void handle(Integer num) { /* ... */ }
  public static void handle(BigDecimal num) { /* ... */ }
  // to handle new types, just add more handle methods...
}

Vous pouvez développer l'idée de gérer de manière générique les sous-classes et les classes qui implémentent certaines interfaces.

Vous pourriez envisager le modèle de chaîne de responsabilité . Pour votre premier exemple, quelque chose comme:

public abstract class StuffHandler {
   private StuffHandler next;

   public final boolean handle(Object o) {
      boolean handled = doHandle(o);
      if (handled) { return true; }
      else if (next == null) { return false; }
      else { return next.handle(o); }
   }

   public void setNext(StuffHandler next) { this.next = next; }

   protected abstract boolean doHandle(Object o);
}

public class IntegerHandler extends StuffHandler {
   @Override
   protected boolean doHandle(Object o) {
      if (!o instanceof Integer) {
         return false;
      }
      NumberHandler.handle((Integer) o);
      return true;
   }
}

et de même pour vos autres gestionnaires. Ensuite, il s'agit d'enchaîner les StuffHandlers dans l'ordre (du plus spécifique au moins spécifique, avec un gestionnaire final de «secours»), et votre code de répartiteur est juste firstHandler.handle(o);.

(Une alternative est, plutôt que d'utiliser une chaîne, d'avoir simplement un List<StuffHandler>dans votre classe de répartiteur et de le faire parcourir la liste jusqu'à ce qu'il handle()renvoie true).

Je pense que la meilleure solution est HashMap avec Class comme clé et Handler comme valeur. Notez que la solution basée sur HashMap fonctionne avec une complexité algorithmique constante θ (1), tandis que la chaîne odorante de if-instanceof-else fonctionne avec une complexité algorithmique linéaire O (N), où N est le nombre de liens dans la chaîne if-instanceof-else (c'est-à-dire le nombre de classes différentes à traiter). Ainsi, les performances de la solution basée sur HashMap sont asymptotiquement plus élevées N fois que les performances de la solution de chaîne if-instanceof-else. Considérez que vous devez gérer différemment les différents descendants de la classe Message: Message1, Message2, etc. Vous trouverez ci-dessous l'extrait de code pour la gestion basée sur HashMap.

public class YourClass {
    private class Handler {
        public void go(Message message) {
            // the default implementation just notifies that it doesn't handle the message
            System.out.println(
                "Possibly due to a typo, empty handler is set to handle message of type %s : %s",
                message.getClass().toString(), message.toString());
        }
    }
    private Map<Class<? extends Message>, Handler> messageHandling = 
        new HashMap<Class<? extends Message>, Handler>();

    // Constructor of your class is a place to initialize the message handling mechanism    
    public YourClass() {
        messageHandling.put(Message1.class, new Handler() { public void go(Message message) {
            //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message1
        } });
        messageHandling.put(Message2.class, new Handler() { public void go(Message message) {
            //TODO: IMPLEMENT HERE SOMETHING APPROPRIATE FOR Message2
        } });
        // etc. for Message3, etc.
    }

    // The method in which you receive a variable of base class Message, but you need to
    //   handle it in accordance to of what derived type that instance is
    public handleMessage(Message message) {
        Handler handler = messageHandling.get(message.getClass());
        if (handler == null) {
            System.out.println(
                "Don't know how to handle message of type %s : %s",
                message.getClass().toString(), message.toString());
        } else {
            handler.go(message);
        }
    }
}

Plus d'informations sur l'utilisation des variables de type Class en Java: http://docs.oracle.com/javase/tutorial/reflect/class/classNew.html

Allez simplement avec l'instance de. Toutes les solutions de contournement semblent plus compliquées. Voici un article de blog qui en parle: http://www.velocityreviews.com/forums/t302491-instanceof-not-always-bad-the-instanceof-myth.html

J'ai résolu ce problème en utilisant reflection(environ 15 ans en arrière dans l'ère pré-générique).

GenericClass object = (GenericClass) Class.forName(specificClassName).newInstance();

J'ai défini une classe générique (classe de base abstraite). J'ai défini de nombreuses implémentations concrètes de la classe de base. Chaque classe concrète sera chargée avec className comme paramètre. Ce nom de classe est défini dans le cadre de la configuration.

La classe de base définit l'état commun à toutes les classes concrètes et les classes concrètes modifieront l'état en remplaçant les règles abstraites définies dans la classe de base.

À ce moment-là, je ne connais pas le nom de ce mécanisme, qui est connu sous le nom de reflection.

Peu d'alternatives supplémentaires sont répertoriées dans cet article : Mapet en enumdehors de la réflexion.