Discussion :

[popo]

Bonjour,

Je dois reprendre un projet avec Unity car mon manager veut que je corrige tous les avertissement remonté par l'analyseur de code.

A l'origine il y a ce bout de code qui provoque l'avertissement CA2000.
- Il s'agit d'une méthode static dans une classe static (si ça peut aider)
- Container est également est de type UnityContainer
- new HierarchicalLifetimeManager() déclenche l'avertissement

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
Container.RegisterType<ICustomRepository<Customer>, CustomRepository<Customer>>(new HierarchicalLifetimeManager());
Container.RegisterType<ICustomRepository<Provider>, CustomRepository<Provider>>(new HierarchicalLifetimeManager());
// Encore une dizaine de lignes sur le même moule

En cherchant un peu sur le net, j'ai découvert un POST de 2010 sur StackOverFlow (réponse qui a été validée et qui est donc normalement bonne) :
https://stackoverflow.com/questions/...rcontrolledlif

J'ai donc adapté mon code en créant la méthode suivante :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
private static void RegisterType<TFrom, TTo>() where TTo : TFrom
{
    var manager = new HierarchicalLifetimeManager();
    try
    {
        Container.RegisterType<TFrom, TTo>(manager);
    }
    catch
    {
        manager.Dispose();
        throw;
    }
}

Et en l'appelant de la manière suivante :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

RegisterType<ICustomRepository<Customer>, CustomRepository<Customer>>();

Au lieu d'avoir plusieurs avertissement CA2000, j'en ai plus qu'un sur ma nouvelle méthode mais toujours sur le new HierarchicalLifetimeManager

Quelqu'un a-t-il une idée pour corriger cet avertissement ?
Merci.

[Noxen]

Est-ce-que ça change quelque chose si tu déplaces l'appel au constructeur dans le try ?

[popo]

J'avais déjà essayé de la manière suivante mais ça n'a rien changé :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
private static void RegisterType<TFrom, TTo>() where TTo : TFrom
{
    HierarchicalLifetimeManager manager = null;
    try
    {
        manager = new HierarchicalLifetimeManager();
        Container.RegisterType<TFrom, TTo>(manager);
    }
    catch
    {
        manager.Dispose();
        throw;
    }
}

[François DORIN]

Une première possibilité serait de préciser explicitement d'ignorer l'avertissement en décorant la méthode avec un attribut

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
[SuppressMessage("Microsoft.Reliability",
                 "CA2000:DisposeObjectsBeforeLosingScope",
                 Justification = "Propriété transférée à Container")]

.

Sinon, peut être (je n'ai pas restesté) une seconde possibilité c'est de créer une méthode qui s'occupe d'instancier l'instance et la retourne. Donc simplement :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
private static HierarchicalLifetimeManager CreateManager()
{
   return new HierarchicalLifetimeManager();
}

car il me semble que l'avertissement CA2000 n'est généré que dans la méthode où l'objet est créé. Donc en déportant la création dans un autre objet, l'avertissement devrait disparaître.

[Kyroon]

Bonjour,

Cet avertissement survient car tu instancie un objet (HierarchicalLifetimeManager) dans ta méthode qui n'est potentiellement jamais disposé (sauf en passant le catch).
- Une solution pourrait être d'instancier ton objet non pas dans la méthode, mais dans la classe (dans le constructeur par exemple) et de le disposer dans le destructeur de la classe (si c'est possible suivant le contexte).
- Je ne connais pas spécialement bien Unity, mais j'ai quand même trouvé ceci : https://docs.microsoft.com/en-us/pre...82(v=pandp.20) qui devrait te permettre de laisser le container faire l'instanciation de l'objet que tu veux passer en paramètre (InjectionConstructor). Ça devrait devrait corriger ton avertissement, car ce n'est plus toi qui fait le new. A tester.

Donc en déportant la création dans un autre objet, l'avertissement devrait disparaître.

C'est vrai, mais ça risque aussi de déporter l'avertissement si ce n'est pas l'objet qui fait la création, qui fait le dispose.

Bonne continuation.

[DotNetMatt]

Ca donne quoi avec un finally ?

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
try
{
    // …
}
catch
{
    throw;
}
finally
{
    manager.Dispose();
}

Comme cela on s'assure que manager est toujours dispose, exception ou pas exception.

[popo]

Une première possibilité serait de préciser explicitement d'ignorer l'avertissement en décorant la méthode avec un attribut
Sinon, peut être (je n'ai pas restesté) une seconde possibilité c'est de créer une méthode qui s'occupe d'instancier l'instance et la retourne. Donc simplement :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
private static HierarchicalLifetimeManager CreateManager()
{
   return new HierarchicalLifetimeManager();
}

car il me semble que l'avertissement CA2000 n'est généré que dans la méthode où l'objet est créé. Donc en déportant la création dans un autre objet, l'avertissement devrait disparaître.

ça ne me génère plus d'avertissement sur le new de cette manière.
Mais l'appel à CreateManager me déclenche le même avertissement.
Dommage.

IL y a un e-book de référence concernant Unity :
Microsoft Pattern & Practice : Unity Container

Dans les exemples de code proposés concernant le HierarchicalLifetimeManager tous font l'instanciation directement "inline" dans l'appel de fonction, ils n'utilisent pas de structure try/catch. J'aurais tendance à penser que cette structure n'est pas nécessaire en l'occurrence, une erreur ne pourrait se produire que dans le constructeur du HierarchicalLifetimeManager, cas qui me paraît improbable, ou dans la méthode Resolve, qui prend peut-être déjà en charge le Dispose. Quand à savoir si ça jouerai sur le rapport de warning je n'en ai aucune idée.

Une première possibilité serait de préciser explicitement d'ignorer l'avertissement en décorant la méthode avec un attribut

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
[SuppressMessage("Microsoft.Reliability",
                 "CA2000:DisposeObjectsBeforeLosingScope",
                 Justification = "Propriété transférée à Container")]

.

ça ma poussé à regarder ce que UnityContainer faisait de cet objet et j'ai découvert ceci :
Dans la méthode RegisterType

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
if (registration.LifetimeManager is IDisposable manager)
    container._lifetimeContainer.Add(manager);

Dans le Dipose de UnityContainer

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

_lifetimeContainer.Dispose();

Dans le dispose de LifetimeContainer

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
foreach (var disposable in disposables)
{
    try
    {
        disposable.Dispose();
    }
        catch (Exception e)
    {
        exceptions.Add(e);
    }
}

Dans la mesure où le container s'occupe d'appeler le Dispose, je vais simplement ignorer l'avertissement.
Merci à vous deux.