Discussion :

[StringBuilder]

Merci pour ces deux réponses, ce sont en effet des raisons qui m'aident à mieux comprendre l'utilité des propriété auto-implémentées par rapport aux champs publiques.

[kheironn]

Merci à Guulh et Bluedeep... voilà des arguments qui ont manqué dans un autre conversation suite à un trollage de poste.
un joli pour eux.

StringBuilder y a eu du mal à comprendre mes arguments en termes de propreté de code, de pattern et d'antipattern (Je code parce que, on ne sait jamais, ça pourra servir en 2080 quand on fera une modif) (dsl pour l'ironie ).
Enfin, personne n'est plus sourd que celui qui ne veut pas entendre.



Je ne reprendrais pas les arguments discutés, mais j'ajoute un lien pour ceux qui veulent un autre pan de la conversation

[Guulh]

Je suis pas sur que c'est tant utilisé. Que tu les vois souvent dans des exemples oui mais dans de vrais projets je suis moins convaincu.
Va falloir faire un sondage

J'avais pas vu ce commentaire, il est écrit tout petit

Je peux t'assurer que sur de vrais projets, de dizaine ou de centaine de milliers de lignes, tout ce qui maximise la maintenabilité, la lisibilité et l'expressivité est prioritaire.
Il peut arriver que certaines features soient ignorées (voire proscrites) parce qu'elles sont mal comprises par la majorité des devs (chépas, le mot-clé yield par exemple) ou rarement utiles (finaliseurs, unsafe, dynamic, ...), mais les propriétés get;set;, c'est un no-brainer comme disent les ricains

Et sur ce genre de gros projets, on utilise aussi souvent (sinon on pleure...) des outils comme CodeRush ou Resharper, qui d'un clic transforme un champ en propriété, ajoute/enlève la variable privée d'un propriété, extrait une interface, ...

Après pour un dev perso sans prétention particulière ni contrainte forte de travail collaboratif et/ou de versioning sur le long terme, ça change rien, en effet.

[champomy62]

Hello,

Ca serait bien de faire un tableau récapitulatif avec avantages & inconvénients.

Vous en pensez quoi ?

[silvinus]

Bonjour,

Juste pour mettre mon petit grain de sel , mais les propriétés servent aussi à "protéger" les données de l'objet. En POO on appel ça l'encapsulation :
http://fr.wikipedia.org/wiki/Encapsu...programmation).
C'est tout pour moi et ça justifie emplement l'existence des champs et propriété.(Après une syntaxe get; set; ou get {} set {} se sera suivant le besoin pour être plus rapide ou faire plus de traitement).

[tomlev]

J'avais pas vu cette discussion quand elle a été créée en janvier, sinon j'aurais aussi mis mon grain de sel
Guulh a très bien résumé les avantages des propriétés. J'ajoute juste un lien vers cet article (traduction d'un article de Jon Skeet) qui traite justement de ce sujet.

[ClaudeBg]

Salut
-----

autant définir explicitement add/remove sur les event

Ben, dans une application multithread, c'est parfois quand même vachement intéressant, ne serait-ce que pour disposer d'un même verrou sur le add, le remove ET le OnEvent...

A+
Claude

[Guulh]

Ben, dans une application multithread,

Hello,

comme disait Dany Boon (dans un autre contexte, certes): il faut lire !

Je ne disais pas que add remove sert à rien; je disais qu'il est inutile (et même nocif pour un projet) de développer un sucre syntaxique tel que le constructeur par défaut, les propriétés/events automatiques, etc. sous le prétexte fallacieux qu'un jour "ça peut servir".

Tant que je suis à poster, pour champomy62: pas besoin d'un tel tableau. Il serait necessaire s'il y avait un choix a faire, mais il n'y en a pas.

[tomlev]

tant que je suis à poster, pour champomy62: pas besoin d'un tel tableau. Il serait necessaire s'il y avait un choix a faire, mais il n'y en a pas.

Il n'y a (généralement) pas vraiment de choix à faire entre propriétés auto-implémentées et propriétés explicites, mais par contre il y bien un choix entre propriétés et champs... Mais je ne pense pas qu'il y ait besoin d'un tableau pour ça ; sauf cas particuliers, si c'est public ça doit être une propriété, et si c'est privé un champ est généralement suffisant.

[ClaudeBg]

Salut
-----

il faut lire !
Je ne disais pas que add remove sert à rien;

J'avais bien compris ton propos, je sais lire

Tu précisais qu'il ne servait à rien d'implémenter add et remove sous prétexte que "ça peut servir", en "englobant" ce cas avec celui des propriétés auto-implémentées. Or, à mon avis (mais bon, je peux faire erreur), ça ne relève pas de la même démarche:

- Laisser les propriétés s'auto-implémenter ne risque de causer aucun défaut fonctionnel, et donc, effectivement, c'est parfaitement inutile de s'amuser à implémenter tout explicitement si ça n'a aucun intérêt, uniquement "parce que ça peut servir", comme tu l'indiques.

- Par contre, laisser les events s'auto-implémenter induit que l'auto-implémentation génère un problème potentiel, surtout si, comme c'est fréquent en programmation objet, on ré-utilise son code dans un autre contexte (ou qu'on le dérive) amenant un contexte multithread (ça arrive plus vite qu'on ne le pense). C'est du reste exactement pour ça que tu conseilles (avec raison) d'utiliser des propriétés auto-implémentées plutôt que des champs (pour la ré-utilisation ou l'évolution), même si dans un cas d'utilisation précis ça peut ne pas avoir de véritable intérêt "logiciel" mais juste "élégant".

J'en déduis (mais ce n'est que mon avis), qu'il n'est pas forcément inutile ou idiot d'implémenter "manuellement" les events dans tout projet important, uniquement "parce que ça peut servir", parce qu'on se met ainsi à l'abri d'un problème futur révélant le défaut en question, difficile à pister puisque asynchrone.

J'ai donc juste voulu différencier les propriétés auto-implémentées des events auto-implémentés, alors que tu me semblais mettre les deux "dans le même sac", si j'ose dire.

On s'est donc juste mal compris, mais j'aurais peut-être du être plus explicite.



A+
Claude

[Guulh]

Ah ok, je comprends mieux ton précédent message

Mais contre quoi te protège le lock ? Me semble avoir déjà entendu parler d'un problème d'accès concurrent relatif aux events, mais je ne me souviens plus des détails; tu aurais un lien vers une page qui décrive le problème ?

[tomlev]

D'autant plus que l'implémentation par défaut des évènements a évolué avec les versions du langage (*)...

@ClaudeBg, es-tu certain d'avoir encore besoin de les implémenter manuellement pour les scénarios multi-thread ? Normalement l'implémentation actuelle est thread-safe, dans la mesure où tu ne peux pas "manquer" un abonnement ou désabonnement par accident. Après, il faut bien sûr faire attention lors du déclenchement de l'évènement ; par exemple, un code comme ça n'est pas thread-safe :

Code :

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

1
2
3
4
5
6
7
public event EventHandler Toto;
 
protected virtual void OnToto()
{
    if (Toto != null)
        Toto(this, EventArgs.Empty);
}

car un handler peut très bien se désabonner entre le test et l'appel des handlers. Il faut donc passer par une variable temporaire :

Code :

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

1
2
3
4
5
6
7
8
public event EventHandler Toto;
 
protected virtual void OnToto()
{
    EventHandler handler = Toto;
    if (handler != null)
        handler(this, EventArgs.Empty);
}

(cf. cet article : http://blogs.msdn.com/b/ericlippert/...and-races.aspx)

Ce code est thread-safe (même avec les anciennes versions de C#, bien que ce n'était pas implémenté de la meilleure façon possible), et je ne vois pas trop pourquoi tu voudrais implémenter manuellement les accesseurs add/remove...


(*) cf. ces articles :
http://blogs.msdn.com/b/cburrows/arc...t-i-locks.aspx
http://blogs.msdn.com/b/cburrows/arc...anges-and.aspx
http://blogs.msdn.com/b/cburrows/arc...g-changes.aspx

[ClaudeBg]

Salut
-----

Mais contre quoi te protège le lock ?

Le lock protège l'accès simultané à remove et add, et donc on a un même objet (forcément) pour remove et add.

Le souci c'est que cet objet n'est pas public et donc on ne sait pas le récupérer pour le "onEvent", sans compter qu'on ignore sur quoi est pris le lock et que ça pourrait être sur "this" (ce qui risque d'entraîner un blocage).

Bref, si on implémente automatiquement, on obtient, dans le meilleurs des cas, un truc comme ceci :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
 
        private EventHandler myEventChanged;
        private objectLock = new Object();
 
        public event EventHandler MyEventChanged
        {
            add
            {
                lock (objectLock)
                    myEventChanged += value;
            }
            remove 
            {
                lock(objectLock)
                    myEventChanged -= value;
            }
        }
 
        private void OnMyEventChanged()
        {
                var handler = MyEventChanged;
            if (handler != null)
                handler(this, EventArgs.Empty);
        }

et dans le pire cas, on aurait même un lock(this)

Donc, on ne sait pas utiliser objectLock dans OnMyEventChanged, puisque objectLock est caché dans l'implémentation automatique.

On peut donc être amené à exécuter l'instruction hander = MyEventChanged alors même qu'on est en train d'exécuter myEventChanged -= value;

Or, je n'ai rien trouvé (mais je peux me tromper) qui garantisse que cette dernière instruction déclenche une série d'instructions monolithiques (qu'on ne peut interrompre). Du coup, on peut très bien assigner handler pendant que le délégué de l'évènement est en cours de modification non terminée.

En créant soi-même add et remove, on empêche ce problème, puisque le verrou "objectLock" devient lui-même accessible, ce qui permet d'écrire :

Code :

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

1
2
3
4
5
6
7
 
        private void OnMyEventChanged()
        {
              Lock (objectLock)
                  var handler = MyEventChanged;
            if (handler != null)
                handler(this, EventArgs.Empty);

Et donc on ne sait pas atteindre l'affectation tant que add et remove sont en cours d'exécution.

Ceci étant dit, il est fort possible que j'ai loupé un épisode , mais dans le doute, j'utilise ce procédé pour mes applications multithread, surtout que mon attention a été attirée sur ce point justement par un document présent sur ce site.

tu aurais un lien vers une page qui décrive le problème ?

Oui, dans vos propres tutoriaux : http://tlevesque.developpez.com/tuto...et-evenements/

Il y est dit explicitement :

Si vous voulez être vraiment thread-safe, de façon à ce que quand vous déclenchez l'évènement, vous utilisiez toujours la valeur la plus récente de la variable délégué, tout en vous assurant que les opérations add/remove n'interfèrent pas l'une avec l'autre, vous devez écrire le corps des opérations add/remove vous-même.

Mais, comme je l'ai dit, c'est possible que ce soit devenu obsolète, mais en l'absence de confirmation certaine que ce problème n'existe pas ou plus, je préfère gérer add et remove, surtout qu'un bug asynchrone est une plaie à debugger.

ClaudeBg, es-tu certain d'avoir encore besoin de les implémenter manuellement pour les scénarios multi-thread ?

Non, pas certain du tout.
C'est juste que le point soulevé dans l'article me semble pertinent et que je préfère un excès de prudence qu'un défaut à ce niveau.

Ce code est thread-safe

Oui, à la condition nécessaire que ce qui se trouve dans les méthodes add et remove soient des opérations qu'on ne peut interrompre (et donc que += et -= sur un délégué ne déclenchent pas une méthode comportant plusieurs instructions interruptibles.

Et à condition que l'élément qui s'abonne ne plante pas s'il reçoit un appel à sa méthode d'abonné après s'être désabonné (ce qui peut arriver si le désabonnement survient après l'affectation de handler = ..... . En effet, un abonné peut très bien considérer qu'une fois désabonné à un évènement il n'a plus à traiter sa réception.

Si ces conditions ne sont pas remplies, le "handler = toto" peut fort bien accéder à toto pendant que celui-ci est en cours de modification, donc dans un état indéterminé. Ou alors appeler la méthode d'un objet abonné qui, ne l'étant plus, n'est plus en mesure de la traiter (destruction d'un textbox affichant le message reçu, par exemple).

Sur ce, je retourne sur mon sérialiseur, je n'arrive vraiment pas à trouver la solution.

A+
Claude

[tomlev]

Le lock protège l'accès simultané à remove et add, et donc on a un même objet (forcément) pour remove et add.

L'implémentation par défaut protège déjà contre ce cas, et depuis C# 4, c'est fait sans même utiliser un lock (donc mieux pour les performances)

On peut donc être amené à exécuter l'instruction hander = MyEventChanged alors même qu'on est en train d'exécuter myEventChanged -= value;

Oui mais ça n'a pas d'importance (cf. plus loin)

Or, je n'ai rien trouvé (mais je peux me tromper) qui garantisse que cette dernière instruction déclenche une série d'instructions monolithiques (qu'on ne peut interrompre).

Le terme qui convient est "atomique"
Et non, ce n'est pas atomique, puisque c'est un appel à Delegate.Combine ou Delegate.Remove. Par contre, l'affectation du résultat à la variable myEventChanged est bien atomique ; la valeur de myEventChanged reste inchangée tant que Delegate.Combine est en cours d'exécution, et n'est donc jamais dans un état indéterminé.

Oui, dans vos propres tutoriaux : http://tlevesque.developpez.com/tuto...et-evenements/

Il y est dit explicitement :

Oui mais c'est vraiment pour des cas très particuliers... Si quasiment personne ne fait ça, c'est simplement parce que ce n'est généralement pas utile. J'ai vu pas mal de cas d'implémentation manuelle des accesseurs add/remove, mais ce n'était jamais pour des raisons de thread safety.

Mais, comme je l'ai dit, c'est possible que ce soit devenu obsolète, mais en l'absence de confirmation certaine que ce problème n'existe pas ou plus, je préfère gérer add et remove, surtout qu'un bug asynchrone est une plaie à debugger.

Oui mais d'un autre côté, en faisant ça tu compliques pas mal ton code, et tu risques d'introduire d'autres bugs...

Oui, à la condition nécessaire que ce qui se trouve dans les méthodes add et remove soient des opérations qu'on ne peut interrompre (et donc que += et -= sur un délégué ne déclenchent pas une méthode comportant plusieurs instructions interruptibles.

Cette condition n'est pas nécessaire. Le delegate récupéré par handler = MyEventChanged sera toujours dans un état déterminé (vu qu'un delegate est immuable). Par contre il n'aura pas forcément la valeur la plus récente, mais comme expliqué plus loin, on s'en fout.

Et à condition que l'élément qui s'abonne ne plante pas s'il reçoit un appel à sa méthode d'abonné après s'être désabonné (ce qui peut arriver si le désabonnement survient après l'affectation de handler = ..... . En effet, un abonné peut très bien considérer qu'une fois désabonné à un évènement il n'a plus à traiter sa réception.

Non, justement, c'est là que tu fais erreur à mon avis. Si tu lis l'article que j'ai indiqué dans mon poste précédent (http://blogs.msdn.com/b/ericlippert/archive/2009/04/29/events-and-races.aspx), il est dit justement à ce sujet :

event handlers are required to be robust in the face of being called even after the event has been unsubscribed

(si tu ne comprends pas l'anglais, ça donne à peu près ça : les gestionnaires d'évènement doivent être robustes pour le cas où ils sont appelés après le désabonnement de l'évènement)

Donc en gros, ce n'est pas à l'objet qui publie l'évènement de se préoccuper de ça ; c'est celui qui gère l'évènement qui doit tenir compte du fait que le handler peut être appelé après le désabonnement.

Si ces conditions ne sont pas remplies, le "handler = toto" peut fort bien accéder à toto pendant que celui-ci est en cours de modification, donc dans un état indéterminé.

cf. ce que j'ai dit plus haut. L'ajout d'un handler n'est pas atomique, mais l'affectation du delegate oui. Le delegate n'est jamais dans un état indéterminé.


Bref, à mon avis tu te compliques la vie pour rien. Le multithreading, c'est un sujet très complexe, et les concepteurs du langage ont déjà bien réfléchi à la question pour nous en mettant en place les protections adéquates. A moins d'être un expert en programmation concurrente, je ne recommanderais à personne de faire ce que tu fais...

[ClaudeBg]

Salut
-----

L'implémentation par défaut protège déjà contre ce cas,

Oui, ce que je voulais dire c'est que la protection n'est pas "extensible" à l'extérieur (au niveau du onevent).

et depuis C# 4, c'est fait sans même utiliser un lock (donc mieux pour les performances)

Ok, merci pour l'info, même si mon application actuelle n'est pas en C#4. Je ne manquerai pas d'en tenir compte.

Par contre, l'affectation du résultat à la variable myEventChanged est bien atomique ; la valeur de myEventChanged reste inchangée tant que Delegate.Combine est en cours d'exécution, et n'est donc jamais dans un état indéterminé.

D'accord, j'ai maintenant confirmation sur ce point, merci. Ce genre d'infos n'est pas toujours simple à trouver.

Oui mais c'est vraiment pour des cas très particuliers

Ok, mais mon application fait grand usage d'évènements échangés entre threads différents, c'est du à la nature même de mon programme. Et le nombre d'évènements peut être important (proche du millier par seconde)
Donc je préfère me prémunir à ce niveau, surtout que ça ne me coûte rien puisque j'ai peu d'évènements différents.

Oui mais d'un autre côté, en faisant ça tu compliques pas mal ton code, et tu risques d'introduire d'autres bugs...

Je n'ai à faire ça que pour le projet principal de ma solution (composée de beaucoup de projets indépendants chargés en plugins). Donc, ce n'est pas vraiment (pour cette application) lourd à faire, et le risque de bugs en faisant ça est nul (je ne fais que remettre les quelques lignes nécessaires et un lock avant l'affectation ne peut causer aucun bug). Niveau complication, 50 lignes de plus c'est négligeable.

Donc en gros, ce n'est pas à l'objet qui publie l'évènement de se préoccuper de ça ; c'est celui qui gère l'évènement qui doit tenir compte du fait que le handler peut être appelé après le désabonnement.

Là, je suis moins d'accord. On doit s'y conformer pour les évènements Windows, puisque c'est prévu comme ça. Donc, c'est logique que tu préviennes les utilisateurs de la façon de procéder avec des évènements dont ils ne maîtrisent pas le déclenchement.

Par contre, pour ses propres évènements spécifiques à une situation donnée c'est loin d'être obligatoire ni forcément conseillé. Tout dépend du contexte (je ne parle pas d'un évènement d'un simple contrôle perso):

- D'une part, c'est beaucoup plus simple (la preuve) de s'assurer via un verrou que l'abonné ne recevra pas d'évènements après son désabonnement, que de gérer la possibilité de réception d'évènements après le désabonnement au niveau de l'abonné. Je comprends que dans les cas "classiques" on rencontre rarement ce soucis, mais justement tous les cas ne sont pas classiques

- D'autre part, dans l'application que je suis en train d'écrire (par exemple), c'est moi qui fournit les évènements et d'autres vont écrire des plugins qui vont s'y abonner. Je préfère de loin m'assurer dès la génération que des soucis asynchrones provoqués par les désabonnements ne vont pas survenir.

- Enfin, l'un n'empêche pas l'autre: même si on part du principe que l'abonné "n'a qu'à " penser qu'il peut recevoir des évènements même s'il n'est plus abonné (ce que je fais avec les évènements dont je ne suis pas responsable), ça n'empêche pas d'éviter que ce cas ne se produise (double sécurité).

ref, à mon avis tu te compliques la vie pour rien. Le multithreading, c'est un sujet très complexe,

Je connais les difficultés rencontrées en multithreading, beaucoup plus complexe à bien mettre au point qu'il n'y paraît (mes principales applications sont souvent liées à du hardware, et je dois souvent avoir recours au multithread). C'est du reste pourquoi je reste sceptique face à la nouvelle mentalité de parallélisation généralisée pour cause de multicores, où on fait du multithread parfois sans vraiment d'intérêt, mais bon, c'est un autre sujet, je "parasite" déjà assez comme ça celui-ci,

A moins d'être un expert en programmation concurrente, je ne recommanderais à personne de faire ce que tu fais...

Ben, au niveau des évènements déclenchés, je ne fais vraiment rien de complexe: je m'arrange juste pour n'envoyer des évènements qu'à ceux qui sont toujours abonnés. Pour le reste, c'est évident que ce n'est pas simple mais je n'ai aucun autre choix.

Je te remercie pour tes informations, elles me seront très utiles dans le futur, c'est certain.

A+
Claude

[kileak]

Bonjour,

je vous ai lus (et les sujets/tuto connexes) avec attention. J'ai beaucoup appris sur l'usage des propriétés. Merci

J'ai 2 questions :
1- Concernant les propriétés auto-implémentées il est dit que

le compilateur crée un champ de stockage privé et anonyme qui peut être accédé uniquement via les accesseurs

Ok mais ce champ privé n'est pas manipulable dans son code puisque caché. Donc si je comprends bien on ne manipule en interne dans sa classe que la propriété publique. Je n'ai pas une grande expérience (oui je sais :p ) mais avoir un champ privé manipulable me parait intéressant, non ?

2- En appliquant vos conseils et à la lecture de Propriétés (MSDN), je me dis que M. MSDN a déclaré le champ side en public (public double side ; ) et que par conséquent, il n'a pas appliqué la règle qui dit : "pas de champ public !". Vous me direz, ce n'est qu'un exemple, mais bon dans l'esprit, j'ai bon ?

En attendant vos réponses, bon week-end ensoleillé (il était temps) à tous !

[tomlev]

Ok mais ce champ privé n'est pas manipulable dans son code puisque caché. Donc si je comprends bien on ne manipule en interne dans sa classe que la propriété publique.

Oui

avoir un champ privé manipulable me parait intéressant, non ?

Pourquoi ? Le code généré pour une propriété auto va être quelque chose comme ça :

Code :

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

1
2
3
4
5
6
private string _toto;
public string Toto
{
    get { return _toto; }
    set { _toto = value; }
}

Donc au final, que tu accèdes à Toto ou à _toto, ça revient exactement au même ; et ce n'est même pas plus performant d'accéder à _toto, vu que les appels à ces accesseurs seront presque certainement inlinés par le compilateur JIT (i.e. les appels seront remplacés par un accès direct au champ au niveau du code exécutable).

2- En appliquant vos conseils et à la lecture de Propriétés (MSDN), je me dis que M. MSDN a déclaré le champ side en public (public double side ; ) et que par conséquent, il n'a pas appliqué la règle qui dit : "pas de champ public !". Vous me direz, ce n'est qu'un exemple, mais bon dans l'esprit, j'ai bon ?

C'est mal, ils n'auraient pas dû déclarer ce champ en public. Mais bon, c'est pas la première fois que MS ne suit pas ses propres recommandations... "Faites ce que je dis, pas ce que je fais" ^^

[ClaudeBg]

Salut
------

Il y a même une chose dont on n'a pas parlé au sujet de l'intérêt d'une propriété auto-implémentée par rapport à un champs: on peut faire ce genre de chose :

public int Prop {get; private set;}

Qui fournit des possibilités qu'un readonly par exemple, ne permet pas: le beurre et l'argent du beurre pour pas plus de code.

A+
Claude

[kileak]

Limpide. Merci et bonne journée !