Mise en cache d'objets construits avec plusieurs paramètres

Question

J'ai une fabrique qui crée des objets de classe MyClass, renvoyant ceux déjà générés lorsqu'ils existent. Comme j'ai la méthode de création (getOrCreateMyClass) en prenant plusieurs paramètres, quel est le meilleur moyen d'utiliser une carte pour stocker et récupérer les objets?

Ma solution actuelle est la suivante, mais cela ne me semble pas trop clair. J'utilise la méthode hashCode (légèrement modifiée) de la classe MyClass pour construire un int basé sur les paramètres de la classe MyClass, et je l'utilise comme clé de la Map.

import java.util.HashMap; 
import java.util.Map; 

public class MyClassFactory { 

    static Map<Integer, MyClass> cache = new HashMap<Integer, MyClass>(); 

    private static class MyClass { 
     private String s; 
     private int i; 

     public MyClass(String s, int i) { 
     } 

     public static int getHashCode(String s, int i) { 
      final int prime = 31; 
      int result = 1; 
      result = prime * result + i; 
      result = prime * result + ((s == null) ? 0 : s.hashCode()); 
      return result; 
     } 

     @Override 
     public int hashCode() { 
      return getHashCode(this.s, this.i); 
     } 

    } 


    public static MyClass getOrCreateMyClass(String s, int i) { 
     int hashCode = MyClass.getHashCode(s, i); 
     MyClass a = cache.get(hashCode); 
     if (a == null) { 
      a = new MyClass(s, i); 
      cache.put(hashCode , a); 

     } 
     return a; 
    } 

} 

Answer 1

Vous ne devriez vraiment pas utiliser le hashcode comme clé dans votre carte. Le hashcode d'une classe n'est pas destiné à garantir nécessairement qu'il ne sera pas le même pour deux instances non égales de cette classe. En effet, votre méthode hashcode pourrait certainement produire le même hash pour deux instances non égales. Vous avez besoin pour mettre en œuvre equals sur MyClass pour vérifier que deux instances de MyClass sont égales sur la base de l'égalité du String et int qu'ils contiennent. Je recommande également de créer les champs s et ifinal afin de fournir une meilleure garantie de l'immuabilité de chaque instance MyClass si vous l'utilisez de cette façon.

Au-delà, je pense que ce que vous voulez est en fait ici un interner .... c'est quelque chose à garantir que vous ne jamais stocker au plus 1 instance d'un MyClass donné en mémoire à la fois. La solution correcte à cela est un Map<MyClass, MyClass> ... plus précisément un ConcurrentMap<MyClass, MyClass> s'il y a une chance de getOrCreateMyClass être appelé à partir de plusieurs threads. Maintenant, vous avez besoin de créer une nouvelle instance de MyClass afin de vérifier le cache lorsque vous utilisez cette approche, mais c'est inévitable vraiment ... et ce n'est pas un gros problème car MyClass est facile à créer.

Guava a quelque chose qui fait tout le travail pour vous ici: son interface Interner et correspondant Interners usine/classe utilitaire. Voici comment vous pouvez l'utiliser pour mettre en œuvre getOrCreateMyClass:

private static final Interner<MyClass> interner = Interners.newStrongInterner(); 

public static MyClass getOrCreateMyClass(String s, int i) { 
    return interner.intern(new MyClass(s, i)); 
} 

Notez que l'utilisation d'une forte interner sera, comme votre code d'exemple, garder chaque MyClass qu'elle détient en mémoire tant que la interner est en mémoire, que quoi que ce soit d'autre dans le programme a une référence à une instance donnée. Si vous utilisez newWeakInterner à la place, s'il n'y a rien d'autre dans votre programme utilisant une instance MyClass donnée, cette instance sera éligible à la récupération de place, vous aidant à ne pas gaspiller de mémoire avec des instances dont vous n'avez pas besoin.

Si vous choisissez de le faire vous-même, vous devrez utiliser un cache ConcurrentMap et utiliser putIfAbsent. Vous pouvez jeter un coup d'oeil à la mise en œuvre de l'interner fort de Guava pour référence que j'imagine ... l'approche de référence faible est beaucoup plus compliquée cependant.

Answer 2

Votre getOrCreateMyClass ne semble pas ajouter au cache si elle crée.

Je pense que cela ne fonctionnera pas correctement lorsque les hashcodes entrent en collision. Les hashcodes identiques n'impliquent pas des objets égaux. Cela pourrait être la source du bug que vous avez mentionné dans un commentaire.

Vous pourriez envisager de créer une classe Pair générique réelle equals et hashCode méthodes et en utilisant Pair<String, Integer> classe comme la clé de carte pour votre cache.

Edit:

La question de la consommation de mémoire supplémentaire en stockant à la fois une clé Pair<String, Integer> et une valeur MyClass pourrait être mieux traitée en faisant le Pair<String, Integer> dans un champ de MyClass et ayant ainsi une seule référence à la présente objet. Cependant, avec tout cela, vous pourriez avoir à vous soucier des problèmes de threading qui ne semblent pas encore être résolus, et qui pourraient être une autre source de bugs.

Et si c'est vraiment une bonne idée dépend de si la création de MyClass est beaucoup plus chère que la création de la clé de la carte.

Une autre Edit:

réponse de ColinD est également raisonnable (et je l'ai upvoted il), tant que la construction de MyClass est pas cher.

Une autre approche qui pourrait être intéressante est d'utiliser une carte imbriquée Map<String, Map<Integer, MyClass>>, ce qui nécessiterait une recherche en deux étapes et compliquerait la mise à jour du cache par un bit.