Discussion :

[VTT85]

Bonjour, je viens de créer un projet qui fonctionne (mon premier programme assez conséquent en tant que débutant).

J'aurais souhaité avoir vos avis sur ma façon de programmer qui je pense est loin d'être exemplaire. Si vous aviez des conseils à me donner, ce serait vraiment cool en tout cas
Le programme est compresser en pièces jointes

Merci d'avance à la superbe communauté developpez.

Valentin.

[PixelJuice]

Bonjour,

J'ai survolé ton code. Bon le problème c'est que tu débutes, donc il y a énormément de chose qui ne vont pas (ce qui est tout à fait normal, il faut apprendre a coder tout court avant d'apprendre a bien coder).

Donc difficile de faire une review de code sans te renvoyer directement sur un bouquin ou un tutoriel concernant les DO / DON'T du code. Je te conseille de continuer ton apprentissage, tout en assimilant un par un chaque bonne habitude, pour que tu ne sois pas submergé et que tu vois l'utilité de la chose en question.

Néanmoins, voici ce qui améloriable (non exhaustif):

• Ton programme est très procédurale, de plus tu n'as qu'une seule classe qui n'est la que pour stocker des méthodes statiques. C'est pas un mal en soi mais vu que tu codes en C#, autant profiter a fond de la programmation orienté objet.
  Ça te permet aussi de bien séparer l'interface du fonctionnement de ton logiciel, ce qui est vitale. Dans un programme bien fait, dans le code de ta fenêtre, il ne devrait avoir que des appels tout bête a des méthodes.
  
  
• Beaucoup de lignes pourraient être remplacer par l'appel d'une méthode. Exemple :
  
  

  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
          public void ligneRound(int w)
          {
              switch (w)
              {
                  case 1:
                      remplirCarre(1);
                      break;
                  case 2:
                      remplirCarre(2);
                      break;
                  case 3:
                      remplirCarre(3);
                      break;
                  case 4:
                      remplirCarre(4);
                      break;
                  case 5:
                      remplirCarre(5);
                      break;
              }
          }

  Ici le switch case est inutile, et comme tu vérifies déjà que z boucle entre 1 et 5, hop :
  
  

  Code :

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

  1
  2
  3
  4
          public void ligneRound(int w)
          {
              remplirCarre(w);
          }

  Du coup, la méthode ligneRound deviens inutile, autant appelée remplirCarre directement.
  
  
• Il faut toujours avoir des bons noms de variables, et qui soient assez parlant, exemple :
  
  

  Code :

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

  1
  2
  3
          public int k = 1;
          public int z = 1;
          public int q = 1;

  Je suis obligé de regarder ton programme pour savoir ce que ces variables font / sont.
  
  Pareil pour ce morceau de code :
  
  

  Code :

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

  1
  2
  3
  4
  5
  6
          public void ChoixCouleur(byte a, byte b, byte c, int round)
          {
              SolidColorBrush mySolidColorBrush = new SolidColorBrush();
              mySolidColorBrush.Color = Color.FromRgb(a, b, c);
              // Etc ...
           }

  a, b et c ce n'est vraiment pas représentatif, pourquoi ne pas utilisé tout simplement
  
  

  Code :

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

  1
  2
  3
  4
  5
  6
          public void ChoixCouleur(byte r, byte g, byte b, int round)
          {
              SolidColorBrush mySolidColorBrush = new SolidColorBrush();
              mySolidColorBrush.Color = Color.FromRgb(r, g, b);
              // Etc ...
           }

  Certes, ce sont des détails mais dans de très gros projets, tout ce qui peut t'éviter de te prendre la tête ou perdre du temps inutilement a retrouver a quoi sert telle et telle variable est bon a prendre.
  
  
• Tu utilises le mot clé out la ou un simple type de retour a la méthode était suffisant. L'utilisation de ce mot clé est censé être assez rare (certains le considère même comme un défaut)
  
  

  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
          private void Aléatoire(out string cr, int num)
          {            
              string couleur = "";
              switch (num)
              {
                  case 1:
                      couleur = "#FFFF0000";
                      break;
                  case 2:
                      couleur = "#FF008000";
                      break;
                  // etc ...
              }
              cr = couleur;
          }

  deviens
  
  

  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
          private string Aléatoire(int num)
          {            
              string couleur = "";
              switch (num)
              {
                  case 1:
                      couleur = "#FFFF0000";
                      break;
                  case 2:
                      couleur = "#FF008000";
                      break;
                  // etc ...
              }
              return couleur;
          }

• CCouleur est instanciable, alors que comme dit plus haut, elle ne sert qu'a stocker des méthodes indépendantes, typiquement cette classe devrait être static.
  

Bref, il y a d'autres choses a dire mais je pense que tu as compris, produire un code de qualité c'est un gros morceau.

Une très bonne habitude a prendre dès maintenant, c'est de refactoriser ton programme, ce qui te permet de nettoyer ton code, de le rendre plus présentable, performant et compréhensible. C'est un excellent moyen de s’améliorer. Attention a le faire uniquement quand le code marche et est terminé, c'est largement plus facile et comme ça tu ne tombes pas dans l'optimisation prématurée.

Je pense que tu peux facilement diviser le nombre de lignes de ton programme par 2.

C'est déjà très bien d'avoir fait un projet comme ça, bonne chance !

[VTT85]

Merci beaucoup PixelJuice d'avoir pris le temps d'analyser mon programme.

Je suis en cours de factorisation du programme, je pense que tu as raison c'est une très bonne méthode pour prendre les bonnes habitudes dans le codage en C#.

J'ai deux questions à ce sujet :

- 1 : Quand tu me dis de faire ça :

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
private string Aléatoire(int num)
        {            
            string couleur = "";
            switch (num)
            {
                case 1:
                    couleur = "#FFFF0000";
                    break;
                case 2:
                    couleur = "#FF008000";
                    break;
                // etc ...
            }
            return couleur;
        }

Je ne comprends pas car nous ne pouvons pas retourner une valeur string alors que nous déclarons une méthode en int, le logiciel me met "Comme 'CCouleur.Aléatoire(int)' retourne void, un mot clé return ne doit pas être suivi d'une expression d'objet" c'est la que je ne comprends pas comment utiliser un return avec une méthode en int. Et de plus, comment l'utiliser par la suite, la variable string couleur ne peut pas être réutilisée ensuite si ?

-2 : Quand j'appelle cette méthode :

Code :

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

corrige11.Fill = green;

et celle-ci par exemple :

Code :

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

corrige12.Fill = green;

Ne serait-il pas plus judicieux de faire "corrige1[i].Fill = green;" ?

Je ne sais pas comment appeler une méthode au dépend d'une variable ?
Pourriez-vous m'éclaircir dessus svp

Merci beaucoup de prendre le temps de m'aider !
Valentin

[VTT85]

Oups, petite erreur de ma part , je n'avais pas vu que la méthode Aléatoire était déclarée en string et non en void.

Ok je suis d'accord pour effectuer ceci mais lorsque nous voulons retourner plusieurs variables string comme ceci est-il possible ?

[WDKyle]

Oups, petite erreur de ma part , je n'avais pas vu que la méthode Aléatoire était déclarée en string et non en void.

Ok je suis d'accord pour effectuer ceci mais lorsque nous voulons retourner plusieurs variables string comme ceci est-il possible ?

Bonjour,

Utiliser un Tuple

https://blog.cellenza.com/csharp/nou...-c-7-1-tuples/

[VTT85]

Bonjour,

Super les Tuple, je trouve ça excellent merci beaucoup WDKyle !

Juste une seconde question qui était celle-ci :

Quand j'appelle cette méthode :

Code :

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

corrige11.Fill = green;

et celle-ci par exemple :

Code :

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

corrige12.Fill = green;

Ne serait-il pas plus judicieux de faire "corrige1[i].Fill = green;" ?

Je ne sais pas comment appeler une méthode au dépend d'une variable ?
Cela me permettrait d'économiser un très grand nombre de lignes.
Pourriez-vous m'éclaircir dessus svp