A propos de l’implémentation de l'interface INotifyPropertyChanged

Version imprimable

11/10/2011, 12h00
Immobilis

A propos de l’implémentation de l'interface INotifyPropertyChanged
Salut,

Ce post juste pour attirer votre attention sur un bout de code qui devrait être adopté par beaucoup.

L’implémentation de l'interface INotifyPropertyChanged implique d'informer/transmettre le nom de la propriété qui a changé à une méthode qui va lever l'évènement.

Cela se fait généralement en passant simplement le nom de la propriété sous la forme d'une chaîne. Cela m'a toujours choqué, car absolument pas fiable!

Heureusement, il existe des méthodes pour remédier à ce risque de bug:
- http://blog.decarufel.net/2009/07/ho...d-type_22.html
- http://www.wpftutorial.net/INotifyPropertyChanged.html
A+

PS: cela a aussi été discuté sur le forum WPF: http://www.developpez.net/forums/d10...mplementation/

Oui, c'est une technique qu'on voit pas mal depuis quelques années... c'est effectivement assez élégant, mais il y a au moins 2 inconvénients :

il faut avoir C# 3 ou plus récent

c'est beaucoup plus lourd en termes de performance. Exemple avec 1000000 itérations :

Citation:

String: 00:00:00.0083548
Lambda: 00:00:03.4212166

Soit 400 à 500 fois plus lent en moyenne...
Le code du test :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
static void Main()
{
    Foo f = new Foo();
 
    // JIT warm-up
    f.Bar = 42;
    f.Baz = 42;
 
    int N = 1000000;
 
    var sw = new Stopwatch();
    sw.Start();
    for (int i = 0; i < N; i++) f.Bar = i;
    sw.Stop();
    Console.WriteLine ("String: {0}", sw.Elapsed);
 
    sw.Reset();
 
    sw.Start();
    for (int i = 0; i < N; i++) f.Baz = i;
    sw.Stop();
    Console.WriteLine ("Lambda: {0}", sw.Elapsed);
}
 
class Foo : INotifyPropertyChanged
{
    private int _bar;
    public int Bar
    {
        get { return _bar; }
        set
        {
            _bar = value;
            OnPropertyChanged("Bar");
        }
    }
 
    private int _baz;
    public int Baz
    {
        get { return _baz; }
        set
        {
            _baz = value;
            OnPropertyChanged(() => Baz);
        }
    }
 
 
    public event PropertyChangedEventHandler PropertyChanged;
 
    protected void OnPropertyChanged<T>(Expression<Func<T>> expr)
    {
        var memberExpr = (MemberExpression)expr.Body;
        OnPropertyChanged(memberExpr.Member.Name);
    }
 
    protected virtual void OnPropertyChanged(string propertyName)
    {
        var handler = PropertyChanged;
        if (handler != null)
            handler(this, new PropertyChangedEventArgs(propertyName));
    }
}

11/10/2011, 12h29
Immobilis

Il faut trouver un juste milieu. Je comprend la raison des 100000 itérations, mais c'est un peu extrême.

Cela reste un choix entre risque de bug/maintenance et performances.

Est-ce qu'on ne pourrait pas obtenir de meilleurs résultats en utilisant la reflection pour obtenir le nom de la propriété?

A+
11/10/2011, 12h49
Invité

Bonjour,

Franchement moi je préfère de loin passer par les chaines de caractères mais pas n'importe comment. J'utilise toujours la même méthode que j'ai avancée dans le post de la discussion que j'avais lancée il y a quelques mois (que tu as d'ailleurs mis en PS dans ton post).

La méthode consiste à utiliser les "Magic Strings" qui sont des constantes stockant les noms des propriétés et du coup si le nom d'une des propriétés change, on a juste un seul endroit où modifier renseigner le nom correct ( ;) oui je sais on peut aussi oublier de le faire). Ne surtout pas oublier que je perds rien en terme de performance et je diminue le risque d'oubli en cas de refactoring.
11/10/2011, 13h23
tomlev

Citation:

Envoyé par Immobilis

Est-ce qu'on ne pourrait pas obtenir de meilleurs résultats en utilisant la reflection pour obtenir le nom de la propriété?

Bah je vois pas trop comment en fait... et de toutes façons la réflexion est très couteuse en terme de performance.

Après, c'est sûr que les chaines de caractères c'est pas idéal, mais avec des bons outils de refactoring on risque moins de créer des bugs. Par exemple, Resharper détecte ce genre de choses et propose de modifier la chaine quand on renomme la propriété.

Et pour ce qui est de l'écriture initiale, j'utilise des snippets de code, ce qui fait que je ne risque pas de mal taper le nom de la propriété.
11/10/2011, 13h30
Samuel Blanchard

Je ne suis pas fan de l'utilisation de la Réflection à tout va, notamment pour le coup temps que cela peut induire (cf Tomlev).
Tout comme Dr House ;) j'utilise des constantes de chaine.
Avec un petit snippet c'est encore mieux !

http://blog.naviso.fr/wordpress/?p=640
11/10/2011, 20h35
GuruuMeditation

+1, ke ne suis pas trop fan de Reflection et autres ExpressionTree dans ce cas (Comme Tomlev j'ai fait des stats il y a quelques mois et les perfs etaient tout aussi bof bof). Je ne suis pas fan de "magic strings" non plus, mais entre deux maux, il faut choisir le moindre. :?
11/10/2011, 20h56
tomlev

Sans aller jusqu'au mythique infoof, un opérateur nameof rendrait bien service...
12/10/2011, 08h48
GuruuMeditation

Citation:

Envoyé par tomlev

Sans aller jusqu'au mythique infoof, un opérateur nameof rendrait bien service...

Oui. Ou alors j'utilise parfois AOP avec Postsharp. Dans le même genre is y le le NotifypropertyWeaver : http://code.google.com/p/notifypropertyweaver/
12/10/2011, 22h45
Immobilis

Sans vouloir chipoter, je me permet de relativiser sur l'inconvénient de la performance[quote=tomlev;6287248]

Citation:

Envoyé par tomlev

c'est beaucoup plus lourd en termes de performance. Exemple avec 1000000 itérations :

Soit 400 à 500 fois plus lent en moyenne...

C'est certain que si on me propose de gagner 500 fois plus je serai content. Cependant, j'avais pas bien vu le nombre. Un millions d'itérations c'est beaucoup. Cela signifie qu'il faut en moyenne 0,0000034212166 secondes pour une notification. Tout de suite cela me parait moins interessant de gagner 500 fois 0,000003 cents ;) Je serai curieux de connaître quelle proportion du temps d'execution d'une page cela occupe comparé à la fréquence à laquelle l'évènement est levé.

Je sais bien qu'il faut faire attention aux perfs, mais vous pensez vraiment que c'est un goulot d'étranglement?

A+
13/10/2011, 01h01
tomlev
Dans le cas général, non, ce ne sera effectivement pas un bottleneck pour les perfs... Mais si tu fais un traitement de masse qui met à jour plein de propriétés sur plein d'objets, là ça aura peut-être un impact non négligeable.

Je mentionnais le problème potentiel de performance parce que c'est important de ne pas l'oublier, mais je ne suis pas du tout opposé à cette solution, d'ailleurs il m'arrive de l'utiliser. La lib Dvp.NET fournit d'ailleurs quelques éléments pour l'utiliser ; par exemple la méthode d'extension Raise :
Code:

1 2 3 4 5 6 7 8 9 10 private int _bar; public int Bar { get { return _bar; } set { _bar = value; PropertyChanged.Raise(() => Bar); } }
14/10/2011, 10h03
chrisdot

Citation:

Envoyé par Immobilis

Je sais bien qu'il faut faire attention aux perfs, mais vous pensez vraiment que c'est un goulot d'étranglement?

+1 !

Dans la prochaine version de mvvmlight (v4), les lambdas seront possibles:

Citation:

Light reflection is used to get the name. This supports Intellisense and can easily be refactored.

Pour moi, le facteur déterminant, c'est le support du refactoring ! Je suis quasi certain de devoir renommer une propriété sur 2 en général....
Ensuite si les perfs le nécessite, bien sûr suis prêt à faire autrement (mais en dans la grande majorité des cas ça le fait très bien).

A+
14/10/2011, 21h50
GuruuMeditation

Citation:

Envoyé par chrisdot

Dans la prochaine version de mvvmlight (v4), les lambdas seront possibles:

Oui effectivement, il m'en avait parlé au MIX11. Par contre, celui sans lambda (le RaisePropertyChanged()) est abandonné, je pense, car il a besoin du ficher DBG (il utilise le stackframe, en fait).
14/10/2011, 23h57
tomlev

Citation:

Envoyé par GuruuMeditation

Par contre, celui sans lambda (le RaisePropertyChanged()) est abandonné, je pense, car il a besoin du ficher DBG (il utilise le stackframe, en fait).

Tu veux dire le fichier PDB ? Il n'y a pas besoin de ça pour avoir des infos sur la Stack, d'ailleurs quand tu as une exception sur du code Release tu as quand même les infos sur la stack.

Par contre cette technique est pas très fiable, parce que les accesseurs de la propriété peuvent être inlinés par le JIT, ce qui fausse la pile. On peut empêcher l'inlining avec un attribut, mais s'il faut écrire cet attribut pour chaque propriété ça annule tout le bénéfice de cette technique
15/10/2011, 09h12
GuruuMeditation

Citation:

Envoyé par tomlev

Tu veux dire le fichier PDB ? Il n'y a pas besoin de ça pour avoir des infos sur la Stack, d'ailleurs quand tu as une exception sur du code Release tu as quand même les infos sur la stack.

Par contre cette technique est pas très fiable, parce que les accesseurs de la propriété peuvent être inlinés par le JIT, ce qui fausse la pile. On peut empêcher l'inlining avec un attribut, mais s'il faut écrire cet attribut pour chaque propriété ça annule tout le bénéfice de cette technique

Oui, sorry, PDB :)
Je ne sais plus quel était le problème exact, mais si on enlevait ce fichier, ça ne fonctionnait plus. Problème qui n'avait pas été détecté lors des unit tests, parce que le fichier était présent.
Comme tu dis, c'est une technique un peu limite. Dommage, c’était beau mais bon...