4
votes

Comment éviter la duplication de code d'une méthode statique dans plusieurs classes

J'ai plusieurs classes contenant du code dupliqué, en particulier des membres et le plus important une méthode statique qui va créer une nouvelle instance de la classe et renvoyer cette instance: soit une instance précédemment créée enregistrée dans un dictionnaire, soit une nouvelle instance en appelant le constructeur .

Une interface n'est pas une option, car j'ai la méthode statique. J'ai essayé de résoudre le problème en introduisant une classe de base qui implémente cette méthode statique, mais je ne trouve pas de moyen de créer et de renvoyer correctement une classe enfant spécifique.

Voici un exemple de code de la situation actuelle avec la classe A et la classe B montrant du code dupliqué.

public class B
{
    private static readonly IDictionary<string, B> Registry = new Dictionary<string, B>();
    public string Name { get; set; }

    public B(string name)
    {
        this.Name = name;
    }

    public static B GetB(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = new B(instanceName);
            }
            return newInstance;
        }
    }
}

Et puis dans la classe B à nouveau il y a un nom de membre et la méthode GetX ().

public class A
{
    private static readonly IDictionary<string, A> Registry = new Dictionary<string, A>();
    public string Name { get; set; }

    public A(string name)
    {
        this.Name = name;
    }

    public static A GetA(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = new A(instanceName);
            }
            return newInstance;
        }
    }
}

Est-il possible d'éviter ce type de duplication de code en introduisant une classe de base ou de toute autre manière?


6 commentaires

Mabe crée la méthode de base, en acceptant un type générique, puis en crée une instance. voir stackoverflow.com/questions/731452/…


Mais si le dictionnaire est statique, alors vous partagerez le même dictionnaire entre A et B , ce qui peut ne pas être ce que vous voulez.


La visibilité publique du constructeur est-elle prévue?


Pourquoi souhaitez-vous limiter le nombre d'instances en fonction de instanceName ? Est-il autorisé à la fois une instance de A et une instance de B avec le même instanceName ?


Name peut faire partie de la classe / interface de base. Quant à GetX , vous ne pouvez pas éviter de le déclarer, sauf si vous pouvez le rendre générique, alors la classe de base est là où elle devrait normalement être. Vous pouvez toujours déclarer une méthode statique spécifique au type pour appeler cette méthode générique pour le confort de ces utilisateurs de type.


@Spotted En fait, je pensais que cela devait être public. À l'aide de la suggestion Activator.CreateInstance, une exception se produit avec un constructeur privé.


4 Réponses :


1
votes

Êtes-vous à la recherche d'une classe de base générique?

public class A : BaseRegistryGetter<A>
{
    public A(string name) : base(name)
    {
    }

    public static A GetA(string instanceName)
    {
        return BaseRegistryGetter<A>.GetValue(instanceName, s => new A(s));
    }
}

Et puis utilisez-la comme ceci:

public abstract class BaseRegistryGetter<T>
{
    private static readonly IDictionary<string, T> Registry = new Dictionary<string, T>();
    public string Name { get; set; }

    public BaseRegistryGetter(string name)
    {
        this.Name = name;
    }

    public static T GetValue (string instanceName, Func<string, T> creator) {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = creator(instanceName);
            }
            return newInstance;
        }
    }
}

La source pour les maladroits approche pour vous assurer qu'il existe un constructeur de chaîne pour A peut être trouvée ici .


0 commentaires

0
votes

Je pense que cela devrait fonctionner. Vous pouvez l'adapter à vos besoins. De plus, il y avait un bogue dans votre code: vous avez oublié de l'ajouter au Registre lors de la création d'une nouvelle instance.

class Program
{
    static void Main(string[] args)
    {
        A a1 = A.GetInstance("a");
        A a2 = A.GetInstance("aa");
        A a3 = A.GetInstance("a");

        B b1 = B.GetInstance("a");
        B b2 = B.GetInstance("aa");
        B b3 = B.GetInstance("a");

        Console.WriteLine(a1 == a2); //false
        Console.WriteLine(a1 == a3); //true

        Console.WriteLine(b1 == b2); //false
        Console.WriteLine(b1 == b3); //true

        Console.ReadKey();
    }
}

public class A : Generic<A>
{
    public A(string name)
        : base(name)
    {
    }
}

public class B : Generic<B>
{
    public B(string name)
        : base(name)
    {
    }
}

public abstract class Generic<T> where T : Generic<T>
{
    private static readonly IDictionary<string, T> Registry = new Dictionary<string, T>();
    public string Name { get; set; }

    public Generic(string name)
    {
        this.Name = name;
    }

    public static T GetInstance(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = (T)Activator.CreateInstance(typeof(T), instanceName);
                Registry.Add(instanceName, newInstance);
            }
            return newInstance;
        }
    }
}


0 commentaires

2
votes

Cela pourrait être un peu plus propre:

public class B: RegistryInstance<B>
{
    public string Name { get; set; }

    public B(string name)
    {
        this.Name = name;
    }
}

public class A : RegistryInstance<A>
{
    public string Name { get; set; }

    public A(string name)
    {
        this.Name = name;
    }
}

public abstract class RegistryInstance<T> where T:class
{
    protected static readonly IDictionary<string, T> Registry = new Dictionary<string, T>();

    public static T GetInstance(string instanceName)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = (T)Activator.CreateInstance(typeof(T), new object[] { instanceName });
                Registry.Add(instanceName, newInstance);
            }
            return newInstance;
        }
    }
}


3 commentaires

Vous avez oublié d'ajouter au Registre lors de la création d'une nouvelle instance. De cette façon, il ne figurera jamais dans le dictionnaire.


@AndersonPimentel, remarqué dès que je l'ai posté. Merci d'avoir remarqué. Bonne réponse d'ailleurs.


Merci @JohnBabb pour cette approche utile.



0
votes

Toutes les autres réponses essaient de résoudre ce problème avec des génériques, mais il se peut que vous ne souhaitiez pas le faire. Premièrement, cela pourrait être une restriction inutile plus loin qui pourrait finir par causer des problèmes de variance. Deuxièmement, cela ne résout qu'un seul niveau d'héritage, s'il y en a plus, vous êtes à nouveau coincé avec le même problème:

public class A: Base
{
    public A(string instanceName)
        :base(instanceName)
    {
    }
    public static A GetA(string instanceName)
        => GetBase(instanceName, () => new A(instanceName)) as A;
}

public class B: Base
{
    public B(string instanceName)
        :base(instanceName)
    {
    }
    public static B GetB(string instanceName)
        => GetBase(instanceName, () => new B(instanceName)) as B;
}

Il existe des solutions générales sans l'utilisation de génériques qui n'impliquent qu'une petite duplication de code . L'un pourrait être le suivant:

public class Base
{
    static readonly IDictionary<string, Base> Registry = 
        new Dictionary<string, Base>();

    protected static Base GetBase(string instanceName,
                                  Func<Base> creator)
    {
        lock (Registry)
        {
            if (!Registry.TryGetValue(instanceName, out var newInstance))
            {
                newInstance = creator();
            }   

            return newInstance;
        }
    }

    //...
}

Et maintenant, vos types dérivés peuvent implémenter une méthode déléguée fortement typée:

 class Base<T> { ... }
 class A: Base<A> { ... }
 class B: A { //How does the generic base class help? }


2 commentaires

Avez-vous oublié d'ajouter à Registry dans votre premier exemple de code? Le Registry pourrait-il être un ConcurrentDictionary (peut-être où la valeur est Lazy )?


@sinatr Désolé pour le désordre, j'ai vraiment besoin d'aller me coucher. C'est plus ou moins ce que j'avais en tête. Et oui, une autre option est de rendre Register générique et d'appeler Activator.CreateInstance mais cela repose sur la réflexion ...