La réflexion dans une classe de base est-elle une mauvaise idée de conception?

Question

En général, la réflexion dans une classe de base peut être utile et utile, mais j'ai ici un cas où je suis entre un rock et un endroit difficile ... Utilisez Reflection, ou exposer des classes publiques quand elles sont vraiment devrait être privé sémantiquement parlant (c'est-à-dire que tout le monde ne devrait pas être capable de les utiliser). Je suppose un code est en ordre ici:

public abstract class SingletonForm<TThis> : Form 
    where TThis : SingletonForm<TThis> 
{ 
    private static TThis m_singleton; 
    private static object m_lock = new object(); 
    private static ISingletonFormFactory<TThis> m_factory; 

    protected SingletonForm() { } 

    public static TThis Singleton 
    { 
     get 
     { 
      lock (m_lock) 
      { 
       if (m_factory == null) 
       { 
        foreach (Type t in typeof(TThis).GetNestedTypes(BindingFlags.NonPublic)) 
        { 
         foreach (Type i in t.GetInterfaces()) 
         { 
          if (i == typeof(ISingletonFormFactory<TThis>)) 
           m_factory = (ISingletonFormFactory<TThis>)Activator.CreateInstance(t); 
         } 
        } 

        if (m_factory == null) 
         throw new InvalidOperationException(string.Format(
          CultureInfo.InvariantCulture, 
          "{0} does not implement a nested ISingletonFormFactory<{0}>.", 
          typeof(TThis).ToString())); 
       } 

       if (m_singleton == null || m_singleton.IsDisposed) 
       { 
        m_singleton = m_factory.GetNew(); 
       } 

       return m_singleton; 
      } 
     } 
    } 
} 

Maintenant, ce code fonctionne pour moi, mais est-ce une bidouille horrible et/ou une mauvaise idée? L'autre option passe dans le type Factory en tant que paramètre de type, mais en raison des limites de visibilité, la classe Factory doit être publique, ce qui signifie que n'importe qui peut l'appeler pour créer des instances alors qu'elles ne le devraient pas.

Answer 1

Lorsque vous traitez avec des génériques, vous devez fréquemment utiliser la réflexion. À cet égard, je pense que vous allez bien. Cela dit, je vois deux formes d'odeur de code ici. Ils peuvent être dus à l'assainissement de code, cependant, je vais juste commenter:

D'abord, votre propriété statique est d'un élément générique. Je suis sûr à 99,999% que cela ne sera même pas compilé. Si c'est le cas, c'est une mauvaise forme. Deuxièmement, vous semblez renvoyer une nouvelle instance pour chaque appel à Bar. Ceci est également considéré comme une mauvaise forme pour un getter. Je voudrais plutôt avoir une méthode appelée CreateBar() ou quelque chose de similaire.

Answer 2

Si c'est possible, j'essaierais d'éviter d'utiliser la réflexion si vous pouvez vous en sortir.

Pour ce faire, vous pouvez essayer d'utiliser un Abstract Factory pattern pour contourner le problème d'exposition directe d'un type d'usine publique, en fonction de votre situation. Comme le dit l'exemple de Wikipedia (en Java), c'est que vous créez une interface d'usine que votre classe d'usine implémente, alors vous avez quelque chose dans votre code qui génère l'usine dont vous avez besoin et la renvoie comme interface d'usine. Une autre façon de procéder consiste à créer une classe abstraite au lieu d'une interface pour votre fabrique abstraite, puis à créer une méthode statique dans cette classe abstraite pour renvoyer l'atelier de type dont vous avez besoin.

Answer 3

Juste un FYI, ce qui suit peut être réduit de

foreach (Type i in t.GetInterfaces()) 
{ 
    if (i == typeof(ISingletonFormFactory<TThis>)) 
     m_factory = (ISingletonFormFactory<TThis>)Activator.CreateInstance(t); 
} 

à

if(typeof(ISingletonFormFactory<TThis>).IsAssignableFrom(t)) 
    m_factory = Activator.CreateInstance(t) as ISingletonFormFactory<TThis>; 

Answer 4

Cette situation est également mieux traitées avec des attributs personnalisés où vous pouvez définir explicitement le type à utiliser.

[AttributeUsage(AttributeTargets.Class, AllowMultiple = false)] 
public sealed class SingletonFactoryAttribute : Attribute 
{ 
    public Type FactoryType{get;set;} 
    public SingletonFormAttribute(Type factoryType) 
    { 
     FactoryType = factoryType; 
    } 
} 

Votre propriété singleton devient

public static TThis Singleton 
{ 
    get 
    { 
     lock (m_lock) 
     { 
      if (m_factory == null) 
      { 
       var attr = Attribute.GetCustomAttribute( 
           typeof(TThis), 
           typeof(SingletonFactoryAttribute)) 
           as SingletonFactoryAttribute; 

       if (attr == null) 
        throw new InvalidOperationException(string.Format(
         CultureInfo.InvariantCulture, 
         "{0} does not have a SingletonFactoryAttribute.", 
         typeof(TThis).ToString())); 

       m_factory = Activator.CreateInstance(attr.FactoryType); 
      } 

      if (m_singleton == null || m_singleton.IsDisposed) 
      { 
       m_singleton = m_factory.GetNew(); 
      } 

      return m_singleton; 
     } 
    } 
}