Concision vs Lisibilité : entre les deux, mon coeur balance

Version imprimable

Hello,

Juste une petite question rapide.

Voici deux bouts de code qui, sauf erreur de ma part, sont fonctionnellement identique.

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
                    For Each p As PROMO_DTO.Product In If(dto.ListType = "INC", dto.Products, dto.ProductsOut)
                        Dim drp As DataRow = product.NewRow
                        drp.Item("BRA_ID") = dto.Brand.Id
                        drp.Item("DEP_ID") = dto.Department.Id
                        drp.Item("DTO_PERCENT") = dto.Percentage
                        drp.Item("PLO_BARCODE") = p.EAN
                        drp.Item("PLO_INCLUSIVE") = If(dto.ListType = "INC", True, False)
                        drp.Item("PLO_COLOR") = p.Color
                        drp.Item("PLO_DESC") = p.Description
                        If p.Price.HasValue Then
                            drp.Item("PLO_PRICE") = p.Price.Value
                        End If
                        drp.Item("PLO_SIZE") = p.Size
                        product.Rows.Add(drp)
                    Next

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
                    If dto.ListType = "INC" Then
                        For Each p As PROMO_DTO.Product In dto.Products
                            Dim drp As DataRow = product.NewRow
                            drp.Item("BRA_ID") = dto.Brand.Id
                            drp.Item("DEP_ID") = dto.Department.Id
                            drp.Item("DTO_PERCENT") = dto.Percentage
                            drp.Item("PLO_BARCODE") = p.EAN
                            drp.Item("PLO_INCLUSIVE") = True
                            drp.Item("PLO_COLOR") = p.Color
                            drp.Item("PLO_DESC") = p.Description
                            If p.Price.HasValue Then
                                drp.Item("PLO_PRICE") = p.Price.Value
                            End If
                            drp.Item("PLO_SIZE") = p.Size
                            product.Rows.Add(drp)
                        Next
                    ElseIf dto.ListType = "EXC" Then
                        For Each p As PROMO_DTO.Product In dto.ProductsOut
                            Dim drp As DataRow = product.NewRow
                            drp.Item("BRA_ID") = dto.Brand.Id
                            drp.Item("DEP_ID") = dto.Department.Id
                            drp.Item("DTO_PERCENT") = dto.Percentage
                            drp.Item("PLO_BARCODE") = p.EAN
                            drp.Item("PLO_INCLUSIVE") = False
                            drp.Item("PLO_COLOR") = p.Color
                            drp.Item("PLO_DESC") = p.Description
                            If p.Price.HasValue Then
                                drp.Item("PLO_PRICE") = p.Price.Value
                            End If
                            drp.Item("PLO_SIZE") = p.Size
                            product.Rows.Add(drp)
                        Next
                    End If

Le premier est plus court mais pas forcément le plus clair au premier abord avec les fonctions If.

Le second est plus long mais on identifie du premier coup d’œil l'utilité de chaque branche du If.

On est bien d'accord que pour si peu, ma question n'a pas de grand intérêt. Mais j'suis du genre perfectionniste et j'ai pas trop l'habitude des fonctions If. Est-ce une bonne pratique ? Si quelqu'un d'autre doit reprendre ce code (ou si je dois revenir dessus dans 3 mois), j'ai des doutes sur la meilleure écriture.

Merci d'avance pour vos avis éclairés :).

19/02/2015, 16h31
Graffito

Pour éviter la répétition de code identique, j'opterai pour la solution1.

Si on veut identifier plus clairement les branches du If, on peut prendre la solution 2 à condition d'écrire une procédure "AddProduct" qui aura comme paramètres "p" et "dto".
19/02/2015, 16h45
Pol63
la 2ème est à bannir, quand on se retrouve avec une bonne partie de code en commun il faut toujours préférer regrouper
la 1ère je suis pas fan (mais ce n'est en rien illisible vu que quand on relie le code d'une boucle on regarde forcément sur quoi ca boucle, et c'est compréhensible)

je ferais plutot comme ca :
Code:

1 2 3 4 5 dim coll as ce_que_cest = dto.ProductsOut if dto.ListType = "INC" Then coll = dto.Products For Each p As PROMO_DTO.Product In coll
mais chacun ses gouts ...
19/02/2015, 16h49
Kropernic

Ouais mais y a un booleen qui varie aussi dans le for each :pastaper:
19/02/2015, 17h06
Pol63

soit tu lui laisses ta syntaxe, soit tu appliques la mienne (en 2 lignes par contre) et dans les 2 cas tu peux faire if coll is dto.Products à la place si tu préfères

Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 Dim source = dto.ProductsOut Dim isInclusive = False If dto.ListType = "INC" Then source = dto.Products isInclusive = True End If For Each p As PROMO_DTO.Product In source Dim drp As DataRow = product.NewRow drp("BRA_ID") = dto.Brand.Id drp("DEP_ID") = dto.Department.Id drp("DTO_PERCENT") = dto.Percentage drp("PLO_BARCODE") = p.EAN drp("PLO_INCLUSIVE") = isInclusive drp("PLO_COLOR") = p.Color drp("PLO_DESC") = p.Description If p.Price.HasValue Then drp("PLO_PRICE") = p.Price.Value drp("PLO_SIZE") = p.Size product.Rows.Add(drp) Next
Item est l'indéxeur par défaut de DataRow du coup on est pas obligé d'utiliser .Item à chaque fois (on pourrait aussi utiliser le dictionary member access operator mais comme c'est peu connu et usité :roll:)
À voir aussi si on peut pas simplifier le Price.HasValue ; en assignant directement p.Price (et donc drp("PLO_PRICE") prendrait la valeur Nothing si le Nullable n'a pas de valeur ?)

19/02/2015, 17h16
Kropernic

Ah j'ai bien la manière de Sehnsucht (comment ça se prononce ?^^).

Pour le membre nullable, c'est la première fois que j'utilise ces machins-là... Du coup, je sais pas trop ce qu'on peut faire ou pas.
J'ai toujours peur qu'en mettant nothing, que ce soit mal traduit coté db... Au moins en ne mettant rien, je suis certain d'avoir NULL. (qui a dit parano ?)

Salut,

Je n'est pas essayé, mais ce genre de code ne fonctionnerait pas ?

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
    Dim dtoProducts As PROMO_DTO.ProductCollection = If(dto.ListType = "INC", dto.Products, dto.ProductsOut)
 
    For Each p As PROMO_DTO.Product In DtoProducts
        Dim drp As DataRow = product.NewRow
        drp.Item("BRA_ID") = dto.Brand.ID
        drp.Item("DEP_ID") = dto.Department.ID
        drp.Item("DTO_PERCENT") = dto.Percentage
        drp.Item("PLO_BARCODE") = p.EAN
        drp.Item("PLO_INCLUSIVE") = (dto.ListType = "INC")
        drp.Item("PLO_COLOR") = p.Color
        drp.Item("PLO_DESC") = p.Description
        If p.Price.HasValue Then
            drp.Item("PLO_PRICE") = p.Price.Value
        End If
        drp.Item("PLO_SIZE") = p.Size
        Product.Rows.Add (drp)
    Next

20/02/2015, 11h16
Kropernic
Citation:
Envoyé par Sehnsucht

Code:

1 2 3 4 5 6 Dim source = dto.ProductsOut Dim isInclusive = False If dto.ListType = "INC" Then source = dto.Products isInclusive = True End If

Item est l'indéxeur par défaut de DataRow du coup on est pas obligé d'utiliser .Item à chaque fois (on pourrait aussi utiliser le dictionary member access operator mais comme c'est peu connu et usité :roll:)
À voir aussi si on peut pas simplifier le Price.HasValue ; en assignant directement p.Price (et donc drp("PLO_PRICE") prendrait la valeur Nothing si le Nullable n'a pas de valeur ?)
Une mini-question pour être sûr de mettre de nommer correctement le concept utilisé.

Le fait de ne pas typer explicitement les variables source et isInclusive, c'est bien ce qu'on appelle l'inférence de type ?

A part économiser quelques caractères, cela a-t-il un autre avantage ?

Hello,

Je profite de la discussion pour savoir comment vous factorisiez le code suivante (la différence entre les deux blocs se situent au niveau du OrderBy vs. [codeinlineOrderByDescending[/codeinline].

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
        If Not IsNothing(PropertyToSort) Then
            If sortorder = Windows.Forms.SortOrder.Ascending Then
                Select Case PropertyToSort
                    Case "Department"
                        data = data.OrderBy(Function(x) x.Department.Code)
                    Case "BrandCode"
                        data = data.OrderBy(Function(x) x.Brand.Code)
                    Case "BrandName"
                        data = data.OrderBy(Function(x) x.Brand.Description)
                    Case "Percentage"
                        data = data.OrderBy(Function(x) x.Percentage)
                    Case "PercentType"
                        data = data.OrderBy(Function(x) x.PercentType)
                    Case "Description"
                        data = data.OrderBy(Function(x) x.Description)
                    Case "Note"
                        data = data.OrderBy(Function(x) x.Note)
                    Case "ListType"
                        data = data.OrderBy(Function(x) x.ListType)
                End Select
            ElseIf sortorder = Windows.Forms.SortOrder.Descending Then
                Select Case PropertyToSort
                    Case "Department"
                        data = data.OrderByDescending(Function(x) x.Department.Code)
                    Case "BrandCode"
                        data = data.OrderByDescending(Function(x) x.Brand.Code)
                    Case "BrandName"
                        data = data.OrderByDescending(Function(x) x.Brand.Description)
                    Case "Percentage"
                        data = data.OrderByDescending(Function(x) x.Percentage)
                    Case "PercentType"
                        data = data.OrderByDescending(Function(x) x.PercentType)
                    Case "Description"
                        data = data.OrderByDescending(Function(x) x.Description)
                    Case "Note"
                        data = data.OrderByDescending(Function(x) x.Note)
                    Case "ListType"
                        data = data.OrderByDescending(Function(x) x.ListType)
                End Select
            End If
        End If

Data est alimenté par une requête linq. Additionnellement, certains objets de data on la propriété Brand à Nothing. Du coup, ça plante en plus d'avoir du code redondant :aie:. J'ai vu passez une news avec une nouvelle version vb ou on ajoute un "?" pour faire un test directement dans l'appel de la propriété... Faut installer un nouveau framework pour profiter de cette nouvelle fonctionnalité ? (noob inside)

C'est quand même assez moche de répéter toutes ces lignes juste pour un descending en plus :(

EDIT : Suis retourné voir la news et le "?." n'est pas encore dispo apparemment :(

20/02/2015, 12h30
Pol63

order by doit demander un predicate(of )
donc tu déclares une variable as ce type
avec le select case tu range dedans la fonction qui tape sur le bon champ
et après tu testes juste dans quel sens pour faire orderby(lafonction) ou orderbydescending(lafonction)

le select case aussi peut etre supprimé (expression ou lambda ou reflection)
20/02/2015, 12h33
Kropernic

Ok, j'vais creuser les predicate(of)... Encore jamais utilisé ça.

Sinon, concernant le problème avec la propriété Brand à Nothing, comment contourner ?

J'ai essayé ceci mais VS n'aime pas (une histoire de type qu'il ne peut pas inférer).

Code:

data = data.OrderBy(Function(x) If(x.Brand Is Nothing, x.Brand))
20/02/2015, 12h45
Kropernic

Réponse à moi-même..

Quand on prend 5 minutes pour réfléchir au message d'erreur sur la raison pour laquelle il ne peut pas inférer un type commun, on comprend qu'il faut lui fournir un objet de type identique pour toutes les branches du if (qui se cache dans la fonction du même nom).

Du coup, en faisant ceci, ça ne plante plus.

Code:

data = data.OrderBy(Function(x) If(x.Brand Is Nothing, 0, x.Brand.Code))

Quel outil incroyable la déduction quand même !
20/02/2015, 12h52
Kropernic

Citation:

Envoyé par Pol63

order by doit demander un predicate(of )
donc tu déclares une variable as ce type
avec le select case tu range dedans la fonction qui tape sur le bon champ
et après tu testes juste dans quel sens pour faire orderby(lafonction) ou orderbydescending(lafonction)

le select case aussi peut etre supprimé (expression ou lambda ou reflection)

J'ai beau chercher sur le net, je ne vois pas d'exemple d'utilisation de OrderBy avec un prédicate.

J'ai bien vu un exemple où la fonction lambda était remplacée par une vrai fonction mais c'est inapplicable à ma situation du fait que data est une liste d'objets de type anonyme... Du coup, pas moyen de typer l'objet que la fonction devrait recevoir.

J'pense que j'vais laisser comme ça pour le moment. C'est pas la mort non plus. (et vu qu'une des KPI de ces dernières années était le nombre de ligne de codes écrites, c'est toujours ça de pris :aie:)
20/02/2015, 13h38
Pol63

ce n'est pas un predicate(of ) comme je le pensais, mais un func(of ) (enfin ca se ressemble)
https://msdn.microsoft.com/fr-fr/lib...vs.110%29.aspx
et un func(of ) tu ne pourras pas me dire que tu n'as jamais utilisé, "Function(x) x.Department.Code" est une instance de type func(of type_de_x)

pour le ? c'est sur vs2015 (qui n'est pas encore sorti)
pour if, il y a aussi iif qui aurait pu marcher, à priori il retourne des object dans les 2 cas donc ca n'aurait pas ralé je pense
ou encore le function(x) peut s'écrire en plusieurs ligne donc tu aurais pu faire if is nothing then return "" else return x.Brand.Code
20/02/2015, 13h46
Pol63
précisions sur le func(of )

Code:

dim f as func(of button,string) = function(b as button)(b.text)

function(b as button)(b.text) est une fonction anonyme (parce qu'elle n'a pas de nom, mais c'est comme une autre fonction)
func(of button,string) est un type, un pointeur vers une fonction
of button, string, ca veut dire que ca prend un bouton en paramètre et que ca retourne un string

le type func(of ) peut s'initialiser avec autre chose qu'une fonction anonyme
Code:

1 2 3 4 5 private function mafonction (b as button) as string end function dim f as new func(of button, string)(addressof mafonction)
il y a le parallèle pour les méthode avec action et sub() à la place de func et function()
Code:

1 2 3 dim a as new action(of string) (sub (s as string) msgbox (s) end sub)
et pour les appels :
Code:

1 2 dim s as string = f.invoke(me.button1) a.invoke(s)