Discussion :

[Iradrille]

Hello,

Je suis recemment tombé sur un bout de code qui m'inquiète un peu :

Code :

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

1
2
3
4
5
6
class Foo {
   public bool Bar {get; set; }
}
List<Foo> foos = ...;
 
foos.Select(a => a.Bar = true).ToList();

C'est équivalent de foos.ForEach(a => a.Bar = true);, en moins lisible.

Mais vu qu'on fait un Select sans récupérer le résultat, y a t'il un risque que le compilo vire l'instruction pour une histoire d'optimisation ?

edit: puis j’imagine qu'une nouvelle List est créée même si non récupérée ?

[WaterTwelve21]

Bonjour ,

La MSDN l'explique trés bien ( Voir section Notes ) https://msdn.microsoft.com/fr-fr/lib...v=vs.110).aspx .

La personne aurai pu trés bien faire

Code :

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

List<Foo> foos = ....Select(a => a.Bar = true).ToList();

Elle à décidé de passer en deux temps.

Et je suis pas d'accord , en quoi c'est moins lisible qu'un forEach ?

[Noxen]

Mais vu qu'on fait un Select sans récupérer le résultat

Le résultat du Select (à savoir un constructeur de requête, et non une liste) est bien récupéré, à travers le ToList() qui est une méthode de "closure". L'appel à ToList() ou ToArray() est même parfois utiliser pour forcer l'exécution des requêtes en profitant de l'effet de bord.

edit: puis j’imagine qu'une nouvelle List est créée même si non récupérée ?

En théorie je dirais oui, en pratique je ne sais pas si le compilateur est suffisamment optimisé pour voir que cette liste n'est pas récupérée et éviter sa création même s'il effectue le parcours de l'ensemble source.

Mais après je suis d'accord que si ce code est techniquement correct il est en revanche sémantiquement faux.

[Noxen]

Code :

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

List<Foo> foos = ....Select(a => a.Bar = true).ToList();

Ce que produit cette ligne de code n'est pas une List<Foo> mais une List<bool> dont les éléments sont tous à true.

Et je suis pas d'accord , en quoi c'est moins lisible qu'un forEach ?

Personnellement je trouve que ça crée une indirection de sens, puisqu'un Select() sert en premier lieu à faire une projection, qui masque l'intention de l'auteur, à savoir parcourir la liste d'objets pour les modifier.

[Iradrille]

Bonjour ,

La MSDN l'explique trés bien ( Voir section Notes ) https://msdn.microsoft.com/fr-fr/lib...v=vs.110).aspx

La doc ne précise pas si l'appel peut être supprimé par optimisation.

Et je suis pas d'accord , en quoi c'est moins lisible qu'un forEach ?

Un Select c'est fait pour sélectionner (ie, foos devrait être une List constante d'objets constants).
C'est comme faire

Code :

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

1
2
3
4
5
6
 
class Foo {
   int Bar { get; set; }
   int Baz { get { Bar = 42; return 0; } // <- valide mais ... c'est pas le rôle d'un getter !
   // un getter n'est pas censé modifier l'objet (hors éventuel besoin de synchronisation ou système de cache, mais c'est particulier)
}

En appelant un getter tu t'attends à ce que l'objet ne soit pas modifié.
De même pour un Select, tu t'attends à ce que la liste ne soit pas modifiée (ni les items quelle contient).

Le résultat du Select (à savoir un constructeur de requête, et non une liste) est bien récupéré, à travers le ToList() qui est une méthode de "closure". L'appel à ToList() ou ToArray() est même parfois utiliser pour forcer l'exécution des requêtes en profitant de l'effet de bord.

Aucun risque que le compilo se dise "la liste résultat n'est pas utilisée, je n'appelle donc pas ToList()" ? -> si ToList n'est pas appelée, le Select n'est pas exécuté.
Ça me semble une optimisation valide et possible, mais vu l'absence de paramètres constants je sais pas comment ça se passe en C#.

[Noxen]

Il ne parait pas légitime de laisser le compilateur ignorer un appel explicite de méthode, même un simple get, qui renvoie une valeur mais ne l'utilise pas, au prétexte d'optimisations. Ce n'est pas parce-qu'une méthode renvoie une valeur qu'elle n'a pas d'autres effets attendus, qui ne se produiraient pas dans ce cas là. De plus la méthode ToList() est expressément présentée dans la msdn (Enumerable.ToList<T>()) comme une technique légitime pour obtenir une évaluation immédiate de la requête. Si optimisation il y avait, et dans la mesure où ces méthodes d'extension sont sous la maîtrise de Microsoft, ce serait de ne pas créer la structure qui emporte la valeur de retour si celle-ci n'est pas utilisée.

À l'inverse, une méthode comme Select() est présentée explicitement comme retournant une objet constructeur qui effectue une évaluation différée de la requête.

[WaterTwelve21]

Vous m'avez convaincu pour la comparaison au ForEach , le Select se la joue double jeu (selection , transformation) .

D'ailleurs à l'utilisation dans l'IDE , je cite visual studio :

Projects each element of a sequence in a new collection
selector : A transform function to apply to each element

De ce que j'ai compris , c'est ce que Noxem explique dans sa derniere déclaration.

une méthode comme Select() est présentée explicitement comme retournant une objet constructeur qui effectue une évaluation différée de la requête.

[ericlm128]

Pour ma part je me demande pourquoi utiliser le select sachant que le foreach existe. Je le trouve plus naturel à lire et ce type d'utilisation de select peux amener des dérives.

L'utilisation du select doit occasionner une consommation de mémoire supplémentaire puisqu'il doit prévoir de retourner la liste projeté contrairement au foreach qui ne retourne rien et qui ne fait que modifier les éléments existants.

Côté performance, en vitesse d'exécution, le foreach est plus de 3 fois plus rapide sur mon poste (300ms vs 1050ms).

Donc je ne vois pas pourquoi utiliser le select dans ce cas précis puisqu'il est :
-Plus lent
-Plus consommateur de mémoire
-Plus long à écrire (ligne de code)
-Moins lisible, intuitif

[Iradrille]

Donc je ne vois pas pourquoi utiliser le select dans ce cas précis puisqu'il est :
-Plus lent
-Plus consommateur de mémoire
-Plus long à écrire (ligne de code)
-Moins lisible, intuitif

Je ne vois pas de raisons non plus, c'est ça été fait comme ça partout dans le projet. Du coup je voulais être sur qu'il n'y ai pas de risques associés.

Merci pour vos réponses, c'est moche, lent, mais ça ne pose pas de problème de résultats qui peuvent changer du jour au lendemain en fonction de l'humeur du compilo.

[François DORIN]

Bonjour,

Si jamais je trouvais ce genre de code dans un projet, je clouerais au pilori le développeur qui a pondu ça (et pourtant, je suis tolérant sur les habitudes de développements de chacun). Ou alors, il faudrait une justification pour une telle écriture, avec le commentaire qui va bien.

Mais ici, il n'y a absolument aucune justification à cela

• un foreach sera plus rapide
• un foreach sera plus lisible
• la méthode select est largement détournée de sa fonction initiale. Ce n'est pas parce qu'on peut planter un clou avec un tournevis qu'il faut utiliser un tournevis !

Sinon, à la question de savoir si le compilateur va supprimer cet appel, la réponse est non. Pour que cela soit possible, il faudrait qu'il soit possible de garantir que le code n'ait pas d'effet de bord. Ce qui n'est pas le cas ici à cause de l'opérateur d'assignation. Et même si l'appel était "ignorable", il faudrait que le code soit simple, pour que les appels de méthode soient "inlinés" (désolé pour cet anglicisme) et que le compilateur puisse supprimer le code superflue. Il faut donc que la closure soit simple (ce qui est le cas ici), mais que l'implémentation de Select et de ToList le soit aussi.