Discussion :

[shadowmoon]

Bonjour

J'ai une fonction qui la "forme" suivante

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
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
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
 
 private string nom_fct(param1, param2)
        {
            string chaine += string.Empty;
 
            switch (param1)
            {
                case val1:
                    chaine += texte1  ;
                    chaine += texte2  ;       
                    break;
 
                case val2:
                    chaine += texte3  ;
                    chaine += texte4  ;
                    break;
 
                case val3:
 
                    if (chaine2.Contains(texte5) == false)
                    {
                        chaine += texte6  ;
                        chaine += texte7  ;
                    }
                    break;
 
                case val4:
 
                    switch (param2)
                    {
                        case val1.1:
 
                            if (chaine2.Contains(texte5) == false)
                            {
                                chaine += texte6  ;
                                chaine += texte7  ;
                            }
 
                            if (chaine2.Contains(texte8) == false)
                            {
                                chaine += texte9  ;
                                chaine += texte10  ;
                            }
 
                            break;
 
                        case val1.2:
                            chaine += texte11  ;
                             chaine += texte12  ;
                            break;
 
                        case val1.3:
 
                            if (chaine2.Contains(texte5) == false)
                            {
                                chaine += texte6  ;
                                chaine += texte7  ;
                            }
 
 
                            if (chaine2.Contains(texte8) == false)
                            {
                                chaine += texte9  ;
                                chaine += texte13  ;
                            }
 
 
                            chaine += texte11  ;
                            chaine += texte14  ;
 
                            break;
 
                        default:
                            break;
                    }
                    break;
 
                case val5:
 
                    switch (param2)
                    {
                        case val1.1:
 
                            if (chaine2.Contains(texte5) == false)
                            {
                                chaine += texte6  ;
                                chaine += texte7  ;
                                chaine += texte9  ;
                                chaine += texte10  ;
                            }
 
                            else
                            {
                                if (chaine2.Contains(texte8) == false)
                                {
                                    chaine += texte9  ;
                                    chaine += texte10  ;
                                }
                            }
 
                            chaine += texte15  ;
                            chaine += texte16  ;
                            break;
 
                        case val1.2:
                            chaine += texte17  ;
                            chaine += texte18  ;
                            break;
 
                        case val1.3:
 
                            if (chaine2.Contains(texte5) == false)
                            {
                                chaine += texte6  ;
                                chaine += texte7  ;
                                chaine += texte9  ;
                                chaine += texte10  ;
                            }
 
                            else
                            {
                                if (chaine2.Contains("texte8") == false)
                                {
                                    chaine += texte9  ;
                                    chaine += texte10  ;
                                }
                            }
 
                            chaine += texte15  ;
                            chaine += texte19  ;
 
                             chaine += texte17  ;
                             chaine += texte20  ;
 
                            break;
 
                        default:
                            break;
                    }
                    break;
 
                default:
                    break;
            }
 
            return chaine;
        }

Je trouve que les deux switch imbriqués l'un dans l'autre, ça fait "sale" et je me demande aussi quel est l'impact sur les performances.

Est-il possible d'améliorer ce code ?

Cordialement

[mermich]

Salut,

En parlant cote perf, les swicth sont au meme niveau que des if else, donc plutots rapides. A mon avis les performances du code fourni sont donc tres bonnes comparees au reste d'une application.

En termes de lisibilite c'est pas top, mais remettre a plat avec un seul niveau sera moins bien niveau perf.

[theMonz31]

l'usage d'un stringbuilder serait nettement plus performant... maintenant, si ce code est appelé intensément, ça vaut le coup d'y passer du temps d'optimisation, mais
si ce n'est pas le cas, il n'y a aucun interêt à s'énerver dessus pour gagner 3 cycles !!!

[mermich]

@theMonz31 de ce que j'ai lu les conditions sont exclusives donc l'usage d'un string builder ne changera quesiment rien.

[Pol63]

on ne peut pas toujours avoir la lisibilité du code (et la maintenabilité, c'est souvent lié) et les meilleures performances

mais de toute façon pour la plupart des applications et la plupart des morceaux de code les meilleures performances on s'en contre fout
attention je ne dis pas qu'il faut coder n'importe comment, un truc de 500ms au lieu de 2ms ca reste un aberration mais 2ms au lieu de 1ms il faut arrêter de chipoter
et du code simple comme celui ci ne peut pas prendre plus d'une milliseconde
là où il faut se poser la question des performances (et donc mesurer, puis se poser la question de la nécessité d'améliorer pondéré entre autre par la fréquence d'utilisation) ce sont les boucles et tout ce qui hors code simple (graphisme, base de données ...)
un if, un switch, un appel de méthode ou une concaténation ca reste en nanosecondes

après les if imbriqués, les switchs (à fortiori imbriqués) et de multiples conditions dans un if ca reste en effet peu lisible, et un des moyens de s'en débarrasser c'est de ne pas en mettre ^^

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
if (param1 == val1) 
{
  ///concat;
  return chaine;
}
 
if (param1 == val2) 
{
  ///concat;
  return chaine;
}
 
/// ...
 
if (param1 != val15)  
{
  return string.empty;
}
 
if (param2 == ....

aucune imbrication, en debug visuel tu peux lire clairement les tests que tu fais dans l'ordre
pas besoin de chercher à quelle ligne on doit sauter ; soit on avance, soit on sort


après il y a des méthodes plus propres, presque aussi lisibles et plus performantes pour transformer ca en quelques lignes de code sans if (reflection, dictionary, fonctions anonymes, expressions ...) mais il faut le détail de ce qu'il y a autour pour voir quelle méthode choisir ...

[DonQuiche]

Je rejoins Pol63 : au diable les performances s'il n'y a aucun problème de performances (99% des cas).

Code :

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

1
2
3
4
5
6
7
string nom_fct(param1, param2)
        {
            if (param1 == val1) return texte1 + texte2;
            if (param1 == val2) return texte3 + texte4;
            if (param1 == val3 && !chaine2.Contains(texte5)) return texte6 + texte7;
 
            // etcétéra avec éventuellement une sous-fonction pour un cas particulier lourd.

Maintenant sur la question des performances, pour ce genre de choses les switch sont la meilleure approche, y compris lorsqu'ils sont imbriqués. Dans certains cas on peut transformer deux switchs imbriqués en un seul (en testant par exemple param1 * constante + param2) mais ce n'est pas forcément plus rapide. Tout dépend des valeurs testées et donc de la traduction du switch en code machine (dans l'idéal une table de sauts de N entrées si on teste N valeurs entières contiguës, ou un dictionnaire si on teste des valeurs complexes, ou des embranchements conditionnels dichotomiques). Si une valeur est beaucoup plus fréquente que les autres il peut être intéressant de la tester en premier, séparément.

Enfin un StringBuilder n'est avantageux qu'à partir de trois chaînes concaténées et de toute façon c'est une des rares optimisations automatiques prises en charge par le compilateur.

[shadowmoon]

l'usage d'un stringbuilder serait nettement plus performant...

un StringBuilder n'est avantageux qu'à partir de trois chaînes concaténées

Su ce point je suis plutôt de l'avis de DonQuiche.

maintenant, si ce code est appelé intensément, ça vaut le coup d'y passer du temps d'optimisation

C'est en effet le cas, je pense même que c'est la fonction la + appelée de tout mon code.

Code :

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

1
2
3
4
5
6
7
8
 
string nom_fct(param1, param2)
        {
            if (param1 == val1) return texte1 + texte2;
            if (param1 == val2) return texte3 + texte4;
            if (param1 == val3 && !chaine2.Contains(texte5)) return texte6 + texte7;
 
            // etcétéra avec éventuellement une sous-fonction pour un cas particulier lourd.

Maintenant sur la question des performances, pour ce genre de choses les switch sont la meilleure approche, y compris lorsqu'ils sont imbriqués. Dans certains cas on peut transformer deux switchs imbriqués en un seul (en testant par exemple param1 * constante + param2) mais ce n'est pas forcément plus rapide. Tout dépend des valeurs testées et donc de la traduction du switch en code machine (dans l'idéal une table de sauts de N entrées si on teste N valeurs entières contiguës, ou un dictionnaire si on teste des valeurs complexes, ou des embranchements conditionnels dichotomiques). Si une valeur est beaucoup plus fréquente que les autres il peut être intéressant de la tester en premier, séparément.

après les if imbriqués, les switchs (à fortiori imbriqués) et de multiples conditions dans un if ca reste en effet peu lisible, et un des moyens de s'en débarrasser c'est de ne pas en mettre ^^

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
 
if (param1 == val1) 
{
  ///concat;
  return chaine;
}
 
if (param1 == val2) 
{
  ///concat;
  return chaine;
}
 
/// ...
 
if (param1 != val15)  
{
  return string.empty;
}
 
if (param2 == ....

aucune imbrication, en debug visuel tu peux lire clairement les tests que tu fais dans l'ordre
pas besoin de chercher à quelle ligne on doit sauter ; soit on avance, soit on sort


après il y a des méthodes plus propres, presque aussi lisibles et plus performantes pour transformer ca en quelques lignes de code sans if (reflection, dictionary, fonctions anonymes, expressions ...) mais il faut le détail de ce qu'il y a autour pour voir quelle méthode choisir ...

Je regarde ce que je peux faire et je vous tiens au courant

Merci pour ces conseils

[DonQuiche]

C'est en effet le cas, je pense même que c'est la fonction la + appelée de tout mon code.

Mais pose t-il un problème de performances ? Sinon, privilégie la lisibilité.

Si tu as un pb alors plutôt que de réorganiser un peu les instructions, ce qui n'aura pas de gros effet, cherche des optimisations plus fondamentales : éviter d'appeler cette fonction dans certains cas (par exemple en mémoisant) ou alors en réutilisant un StringBuilder tout du long (voire en utilisant directement des Array.Copy et en retournant un tableau), ou en parallélisant.

[shadowmoon]

Mais pose t-il un problème de performances ? Sinon, privilégie la lisibilité.

Si tu as un pb alors plutôt que de réorganiser un peu les instructions, ce qui n'aura pas de gros effet, cherche des optimisations plus fondamentales : éviter d'appeler cette fonction dans certains cas (par exemple en mémoisant) ou alors en réutilisant un StringBuilder tout du long (voire en utilisant directement des Array.Copy et en retournant un tableau), ou en parallélisant.

Le code, tel que cité dans mon premier message, n'engendre aucun souci de performance.

J'essaye de l'optimiser et de le rendre plus lisible car je n'ai pas d'autre projet sur lequel travailler.

Celui que je code en ce moment dois être fourni à la "validation" fin octobre. J'ai eu le cahier de recette en avance et tout est ok, aucune erreur en 12 jours de tests.

NB : Je suis employé au service informatique d'un grand groupe bancaire / financier / assureur et l'inertie des décideurs est telle qu'aucun nouveau projet ne démarrera avant l'année prochaine ...

[shadowmoon]

Après des retours du type "c'est bien comme ça" de la part de mes collègues et de mon chef de projet, je ne change pas la structure de mon code.

Merci pour vos conseils, je les appliquerai la prochaine fois