Classes de refactoring utilisant des variables globales

Question

Je travaille sur des classes qui obtiennent une partie de leur configuration à partir de variables globales, par ex.

class MyClass { 
    public void MyClass(Hashtable<String, String> params) { 
     this.foo = GlobalClass.GLOBALVAR.get("foo"); 
     this.bar = GlobalClass.GLOBALVAR.get("bar"); 
     this.params = params; 
    } 
} 

Cela est mauvais pour deux raisons, parle de GLOBALVAR à une base de données pour obtenir certaines variables, ce qui rend vraiment difficile de faire des tests unitaires. L'autre problème est que j'ai plusieurs (douzaines) de classes qui héritent de MyClass, donc je ne peux pas facilement changer la signature du constructeur.

Ma solution actuelle consiste à créer un constructeur par défaut supplémentaire et des méthodes de réglage pour params, foo et bar.

class MyClass { 
    // Other code still here for backwards compatibility. 
    public void MyClass() { 
     // Do nothing much. 
    } 
    public void setParams(Hashtable<String, String> params) { 
     this.params = params; 
    } 
    public void setFoo(Foo foo) { 
     this.foo = foo; 
    } 
    public void setBar(Bar bar) { 
     this.bar = bar; 
    } 
} 

Des idées sur un bon moyen de refactoriser cela, en plus de la façon dont je l'ai fait? Mon autre pensée serait d'utiliser une méthode d'usine, mais j'ai peur de rencontrer des problèmes de substitution polymorphique.

Answer 1

Je pense que je commencerais par faire ce qui suit. Il permet à votre code existant de fonctionner sans modification, et vous permet d'ajouter de nouveaux constructeurs aux sous-classes comme vous le pouvez. Une fois que toutes les sous-classes ont le nouveau constructeur et que tous les appels aux anciens constructeurs ont disparu, vous pouvez vous débarrasser de la classe GlobalClass et des constructeurs qui l'utilisent. Vous pouvez aussi, je l'espère, travailler sur le nettoyage de GLOBALVAR (la classe Car dans mon code).

import java.util.Hashtable; 


class MyClass 
{ 
    private final Foo foo; 
    private final Bar bar; 
    private final Hashtable<String, String> params; 

    public MyClass(final Hashtable<String, String> params) 
    { 
     this(params, GlobalClass.GLOBALVAR); 
    } 

    // added constructor 
    public MyClass(final Hashtable<String, String> params, 
        final FooBar fooBar) 
    { 
     this.foo = fooBar.getFoo(); 
     this.bar = fooBar.getBar(); 
     this.params = params; 
    } 
} 

class MySubClass 
    extends MyClass 
{ 
    public MySubClass(final Hashtable<String, String> params) 
    { 
     super(params); 
    } 

    // added constructor 
    public MySubClass(final Hashtable<String, String> params, 
         final FooBar fooBar) 
    { 
     super(params, fooBar); 
    } 
} 

// unchanged 
class GlobalClass 
{ 
    public static Car GLOBALVAR; 
} 

// added interface 
interface FooBar 
{ 
    Foo getFoo(); 
    Bar getBar(); 
} 

class Car 
    // added implements 
    implements FooBar 
{ 
    private Foo foo = new Foo(); 
    private Bar bar = new Bar(); 

    public Object get(final String name) 
    { 
     if(name.equals("foo")) 
     { 
      return (foo); 
     } 

     if(name.equals("bar")) 
     { 
      return (bar); 
     } 

     throw new Error(); 
    } 

    // added method 
    public Foo getFoo() 
    { 
     return ((Foo)get("foo")); 
    } 

    // added method 
    public Bar getBar() 
    { 
     return ((Bar)get("bar")); 
    } 
} 

// unchanged 
class Foo 
{ 
} 

// unchanged 
class Bar 
{ 
} 

Answer 2

Une légère variation de votre approche serait d'avoir un objet de type GLOBALVAR dans la classe et d'utiliser celui-ci au lieu du réel global (ce refactoring devrait être une simple recherche/remplacement). Vous pouvez définir par défaut la nouvelle variable sur la variable globale réelle et fournir un remplacement pour le test.

Answer 3

Votre classe devrait prendre toutes ses dépendances dans le constructeur. C'est une bonne idée de rendre impossible la création d'une instance de classes invalide ou non initialisée. Rendre foo et bar privé et final, et les définir dans le constructeur.

Answer 4

Je pense que vous devriez introduire une interface pour mettre une couche d'abstraction entre la collection de variables globale et ses consommateurs.

interface GlobalVars { 
    String get(String key); 
} 

Vous devez introduire un constructeur avec une portée limitée, probablement package-private

MyClass(GlobalVars globals, Map<String, String> params) { 
    // create the object 
} 

Et puis fournir public static méthodes d'usine à utiliser ce constructeur.

Avec

public static MyClass newMyClass(Map<String, String> params) { 
    return new MyClass(GlobalClass.GLOBAL_VAR, params); 
} 

cette conception vous pouvez passer dans une mise en œuvre de la maquette GlobalVars dans un test unitaire à partir du même paquet en invoquant explicitement le constructeur.

Addendum: Depuis params semble être un champ obligatoire, je serais certainement faire final et éviter l'approche où vous ajoutez mutateurs pour les écraser.

private final Map<String, String> params; 

De même, faites une copie défensive pour empêcher l3xt h4x.

this.params = Collections.unmodifiableMap(params); 

Answer 5

Cette classe GlobalClass.GLOBALVAR doit être hachée en unités logiques. De cette façon, il serait plus facile de faire des objets simulés pour les tests unitaires. Par exemple, dans mon application de découpe de métal CAO/FAO, j'ai un MaterialList, un SheetSizeList, PartNestingParameters, etc.

Je n'ai pas une énorme liste de choses de variables dans une classe AppParameter géante. Ils pendent tous un objet ShopStandards. Pour le test unitaire impliquant un PartNestingParmeters spécifique, je vais juste aller ShopStandards.PartNestingParmeters = new MockPartNestingParameterTest114(). Le test ne fonctionnera pas en réalisant que les paramètres d'imbrication des pièces sont une maquette. De plus, cela me permet de ne pas avoir à faire des dizaines d'affectations juste pour obtenir la configuration de ShopStandard correctement pour le test.

Nous avons encore plus automatisé où beaucoup de la charge Mock à partir de fichiers enregistrés au cours du test exécuté au cours du développement initial.

Answer 6

Puisque vous indiquez que vous avez la liberté de modifier la hiérarchie des classes.

• Modifiez la base MyClass ctor pour prendre 3 paramètres params, foo et bar. Commentez les références de GlobalVar et cachez simplement les valeurs passées dans le cache
• Compilez ... cela devrait générer un tas d'erreurs de compilation - pas de ctor qui prend 1 paramètre.
• Fixez chacun pour passer dans GlobalVar.get ("foo") et GlobalVar.get ("bar"). Obtenez-le pour construire.
• Affiner maintenant: Réduire les hits à la base de données par charge paresseuse et la mise en cache des valeurs foo et barre. Exposer via une propriété sur GlobalVar.