Est-ce une bonne ou une mauvaise pratique d'appeler des méthodes d'instance à partir d'un constructeur java?

Question

Il y a plusieurs façons d'initialiser des objets complexes (avec des dépendances injectées et la configuration requise des membres injectés), tout cela semble raisonnable, mais présente divers avantages et inconvénients. Je vais vous donner un exemple concret:

final class MyClass { 
    private final Dependency dependency; 
    @Inject public MyClass(Dependency dependency) { 
    this.dependency = dependency; 
    dependency.addHandler(new Handler() { 
     @Override void handle(int foo) { MyClass.this.doSomething(foo); } 
    }); 
    doSomething(0); 
    } 
    private void doSomething(int foo) { dependency.doSomethingElse(foo+1); } 
} 

Comme vous pouvez le voir, le constructeur fait 3 choses, y compris appeler une méthode d'instance. On m'a dit que l'appel de méthodes d'instance à partir d'un constructeur n'est pas sûr car cela évite les vérifications du compilateur pour les membres non initialisés. C'est à dire. J'aurais pu appeler doSomething(0) avant de définir this.dependency, ce qui aurait compilé mais pas travaillé. Quelle est la meilleure façon de refactoriser cela?

1. Faire doSomething statique et passer dans la dépendance explicitement? Dans mon cas, j'ai trois méthodes d'instance et trois champs de membre qui dépendent l'un de l'autre, donc cela semble être beaucoup plus pratique pour rendre ces trois éléments statiques. Déplacez addHandler et doSomething dans une méthode @Inject public void init().Bien que l'utilisation avec Guice soit transparente, il faut une construction manuelle pour être sûr d'appeler init() sinon l'objet ne sera pas entièrement fonctionnel si quelqu'un oublie. En outre, cela expose plus de l'API, qui semblent tous deux mauvaises idées.

2. Wrap une classe imbriquée pour maintenir la dépendance pour vous assurer qu'il se comporte correctement sans exposer API supplémentaire:

   class DependencyManager { 
       private final Dependency dependency; 
       public DependecyManager(Dependency dependency) { ... } 
       public doSomething(int foo) { ... } 
   } 
   @Inject public MyClass(Dependency dependency) { 
       DependencyManager manager = new DependencyManager(dependency); 
       manager.doSomething(0); 
   }

   Cette tractions méthodes d'instance sur tous les constructeurs, mais génère une couche supplémentaire de classes, et quand j'avais déjà intérieur et les classes anonymes (par exemple ce gestionnaire) cela peut devenir déroutant - quand j'ai essayé ceci, on m'a dit de déplacer le DependencyManager dans un fichier séparé, ce qui est également désagréable car il y a maintenant plusieurs fichiers pour faire une seule chose.

Alors, quelle est la meilleure façon de faire face à ce genre de situation?

Answer 1

Josh Bloch dans Effective Java recommande d'utiliser une méthode d'usine statique, bien que je ne trouve aucun argument pour des cas comme celui-ci. Il y a, cependant, un cas similaire dans Java Concurrency in Practice, spécifiquement destiné à empêcher la fuite d'une référence à this du constructeur. Appliqué à ce cas, il ressemblerait à ceci:

final class MyClass { 
    private final Dependency dependency; 

    private MyClass(Dependency dependency) { 
    this.dependency = dependency; 
    } 

    public static createInstance(Dependency dependency) { 
    MyClass instance = new MyClass(dependency); 
    dependency.addHandler(new Handler() { 
     @Override void handle(int foo) { instance.doSomething(foo); } 
    }); 
    instance.doSomething(0); 
    return instance; 
    } 
    ... 
} 

Toutefois, cela peut ne pas fonctionner correctement avec l'annotation DI que vous utilisez.

Answer 2

Ouais, il est en fait illégal, il ne devrait vraiment pas même compiler (mais je crois qu'il fait)

Tenir compte du modèle de constructeur à la place (et maigre vers immuable qui, en termes de modèle de constructeur signifie que vous ne pouvez pas appeler n'importe quel setter deux fois et ne peut appeler aucun setter après que l'objet ait été "utilisé" - appeler un setter à ce point devrait probablement lancer une exception d'exécution).

Vous pouvez trouver les diapositives de Joshua Bloch sur le (nouveau) modèle Builder dans une présentation de diapositives appelé "Effective Java Reloaded: Cette fois, il est pour le Real", par exemple ici:

http://docs.huihoo.com/javaone/2007/java-se/

Answer 3

Vous pouvez utiliser une méthode statique qui prend la dépendance et construit et retourne une nouvelle instance, et marque le constructeur Friend. Je ne suis pas sûr que Friend existe en Java (est-ce que le paquet est protégé?) Ce n'est peut-être pas le meilleur moyen. Vous pouvez également utiliser une autre classe Factory pour créer MyClass.

Modifier: Wow un autre posté a suggéré cette même chose. On dirait que vous pouvez rendre les constructeurs privés en Java. Vous ne pouvez pas faire cela dans VB.NET (pas sûr de C#) ... très cool ...

Answer 4

Il affecte aussi mal l'héritage. Si votre constructeur est appelé dans la chaîne pour instancier une sous-classe de votre classe, vous pouvez appeler une méthode qui est surchargée dans la sous-classe et repose sur un invariant qui n'est pas établi tant que le constructeur de la sous-classe n'a pas été exécuté.

Answer 5

Vous devriez faire attention à l'utilisation de méthodes d'instance dans le constructeur, car la classe n'a pas encore été entièrement construite. Si une méthode appelée utilise un membre qui n'a pas encore été initialisé, eh bien, de mauvaises choses vont arriver.