Comment pourrais-je organiser ce code?

Question

myThread est une fonction qui s'exécute toutes les secondes, elle lit essentiellement certaines données qui doivent être analysées et exécutées. La fonction a beaucoup progressé et il y a plus de 1500 lignes de code comme l'exemple ci-dessous avec beaucoup de [sinon else else] bloque beaucoup de répétitions comme sleep ou SendToChat pour envoyer une commande à la console, et c'est très difficile à maintenir , apportez des modifications, etc. Je voudrais quelques conseils (si possible avec des exemples de code, cela m'aiderait à comprendre la disposition) sur la façon dont je pourrais réécrire cela, je ne suis pas très expérimenté donc je ne suis pas si sûr des possibilités de transformer ce code en un meilleur code pour la maintenabilité et la lisibilité?

De même, n'hésitez pas à commenter les fonctions ou toute autre chose, car cela pourrait m'aider à améliorer d'autres choses qui ne vont pas.

Ceci est juste un échantillon du code et pas tout le code si vous sentez que vous avez besoin d'autres informations du code n'hésitez pas à demander et je posterai une fois que je peux.

PS: ce n'est pas une chose irc.

private void myThread() 
{ 
    while(isRunning) 
    { 
     // SOME PARSED DATA HERE 
     // if (parsedData matchs) do the below stuff 
     Messages received = new Messages 
     { 
      Sent = Convert.ToDateTime(match.Groups[1].Value), 
      Username = match.Groups[3].Value, 
      MessageType = (match.Groups[2].Length > 0) ? MsgType.System : MsgType.Chat, 
      Message = match.Groups[4].Value.Trim(), 
      CommandArgs = match.Groups[4].Value.ToLower().Trim().Split(' ') 
     }; 

     // Get the user that issued the command 
     User user = usersList.Find(x => x.Name == recebe.received.ToLower()); 
     if (user != null) 
     { 
      // different greetings based on acess level 
      if (received.Message == "has entered this room") 
      { 
       if (User == null) 
       { 
        SendToChat("/w " + received.Username + " " + received.Username + " you have no registration."); 
        Thread.Sleep(1000); 
        SendToChat("/kick " + received.Username + " not registered."); 
        Thread.Sleep(1000); 
       } 
       else 
       { 
        string cmd = (user.Access < Access.Vouch) ? 
         "/ann " + user.Access.ToString() + " <" + received.Username + "> has entered the room." : 
         "/w " + received.Username + " " + received.Username + " welcome to our room !"; 
        SendToChat(cmd); 
        Thread.Sleep(1000); 
       } 
      } 
      // Here we filter all messages that start with a . which means it is a command 
      else if (received.Message.StartsWith(".") && user != null) 
      { 
       // here we verify if the user has Access to use the typed command and/or if the command exists 
       if (accessList.Exists(x => x.Access == user.Access && x.HasCommand(received.CommandArgs[0]))) 
       { 
        if (received.CommandArgs[0] == ".say") 
        { 
         SendToChat("/ann " + received.Username + " says: " + received.Message.Substring(received.CommandArgs[0].Length + 1)); 
         Thread.Sleep(1000); 
        } 
        else if (received.CommandArgs[0] == ".command") 
        { 
         string allowedList = string.Empty; 
         int count = 0; 
         foreach (string cmd in listaAccesss.Find(x => x.Access == user.Access).Command) 
         { 
          if (count == 0) 
           allowedList += cmd; 
          else 
           allowedList += ", " + cmd; 
         } 
         SendToChat("/w " + received.Username + " " + received.Username + " you are allowed to use the followed commands: " + permite); 
         Thread.Sleep(1000); 
            } 
        else if (received.CommandArgs[0] == ".vip") 
        { 
         if (received.Command.Count() < 2) 
         { 
          SendToChat("/w " + received.Username + " " + received.Username + ", see an example of how to use this command: .vip jonh"); 
          Thread.Sleep(1000); 
         } 
         else if (received.Command.Count() == 2) 
         { 
          var target = usersList.Find(x => x.Name == received.CommandArgs[1]); 
          if (target == null) 
          { 
           User newUser = new User 
           { 
            Name = received.CommandArgs[1].ToLower(), 
            Access = Access.VIP, 
            Registered = DateTime.Now, 
            RegisteredBy = received.Username.ToLower() 
           }; 

           usersList.Add(newUser); 

           SendToChat("/ann " + user.Access.ToString() + " " + user.Name + " has promoted " + received.CommandArgs[1] + " to VIP."); 
           Thread.Sleep(1000); 
          } 
          else if (target != null && target.Access == Access.VIP) 
          { 
           SendToChat("/w " + received.Username + " " + received.Username + " the user " + target.Name + " already have VIP access."); 
           Thread.Sleep(1000); 
          } 
          else if (target != null && user.Access == Access.HeadAdmin && user.Access < target.Access) 
          { 
           Access last = target.Access; 
           target.Access = Access.Vouch; 

           SendToChat("/ann " + user.Access.ToString() + " " + received.Username + " promoted/demoted the " + last.ToString() + " " + target.Name + " to VIP."); 
           Thread.Sleep(1000); 
          } 
          else if (target != null && target.Access == Access.Vouch) 
          { 
           target.Access = Access.VIP; 
           target.RegisteredBy = user.Name; 

           SendToChat("/ann " + user.Access.ToString() + " " + received.Username + " promoted the vouch of " + target.Name + " to VIP."); 
           Thread.Sleep(1000); 
          } 
          else 
          { 
           SendToChat("/w " + received.Username + " " + received.Username + " you can't register or modify the user " + received.CommandArgs[1] + "."); 
           Thread.Sleep(1000); 
          } 
         } 
        } 
       } 
       else 
       { 
        SendToChat("/w " + received.Username + " " + received.Username + " command not found."); 
        Thread.Sleep(1000); 
       } 
      } 
     } 
     Thread.Sleep(1000); 
    } 
} 

Answer 1

Une bonne façon d'organiser ce code est en utilisant le modèle de conception « Chain of responsibility ».

L'idée est d'avoir une classe pour chaque « commande » qui sait quoi faire, par exemple:

public class NewUserCommand : ConsoleCommand 
{ 
    public override void Process(Context context, string command) 
    { 
     if (context.User != null) 
     { 
      // different greetings based on acess level 
      if (context.Received.Message == "has entered this room") 
      { 
       if (context.User == null) 
       { 
        SendToChat("/w " + context.Received.Username + " " + context.Received.Username + " you have no registration."); 
        Thread.Sleep(1000); 
        SendToChat("/kick " + context.Received.Username + " not registered."); 
        Thread.Sleep(1000); 

        //I could process the request 
        return; 
       } 
      } 
     } 

     //I dont know what to do, continue with the next processor 
     this.Successor.Process(context, command); 
    } 
} 

Vous aurez besoin d'une de ces classes pour chaque commande.

Answer 2

Souvent, lorsque vous voyez une logique conditionnelle complexe (beaucoup de cas/blocs d'autre et les conditions imbriquées), vous devriez considérer refactorisation votre code dans les modèles de conception State ou Strategy (dépend de la situation). Jetez un oeil à ceux pour quelques idées.

MISE À JOUR:
Si vous éprouvez des difficultés sur la façon de factoriser votre code, il y a un grand livre que je lu récemment qui aide à décomposer le processus (couvre tout d'ajouter le contrôle de version, les tests unitaires, d'intégration continue ... parle aussi de diviser itérativement jusqu'à ce que vous soyez capable d'appliquer des choses comme l'injection de dépendance, etc.). Elle ne couvre pas les sujets en profondeur complète, car il y a des livres entiers consacrés à chaque sujet individuel, mais il est un excellent point de départ:
Brownfield Application Development in .Net

Answer 3

Eh bien, pour le début, je pense que vous devez diviser ce énorme code dans plusieurs méthodes. Pour séparer ces parties logiques que vous avez commentées. Vous devriez avoir une méthode pour chaque partie de commentaire (c'est minimum). Essayez de commencer ce:

1. Méthode: // des données analysables ICI // if (parsedData matchs) faire les choses ci-dessous Essayez de faire une méthode de cela.

2. Méthode // Obtenir l'utilisateur qui a émis la commande

3. Méthode // différents messages d'accueil en fonction du niveau de acess

4. Méthode // Ici on filtre tous les messages qui commencent par un. ce qui signifie qu'il est une commande

et ainsi de suite ...

Et vous devez faire des méthodes de parties qui se répètent, vous pouvez donc faire des changements toujours sur un seul endroit, pas throught chercher tout le code et changer chaque paramètre à une nouvelle valeur.

Answer 4

Habituellement, je fais un petit tour qui, bien qu'il ne soit pas une bonne méthode de refactoring, rend le code plus lisible.

Au lieu d'avoir:

if (condition1) { 
    statement1; 
} else if (condition2) { 
    statement2; 
    if (condition3) { 
     statement3; 
    } 
} 

... J'utilise:

if (condition1) { 
    statement1; 
    return; 
} 

if (condition2) { 
    statement; 
    return; 
} 
if (condition2 && condition3) { 
    statement3; 
    return; 
} 

Réduire la complexité du code est la première étape pour le briser. Les prochaines étapes pour que je puisse prendre sont:

• refactoring dans différentes méthodes
• trouver des méthodes similaires, refactoring et ayant des méthodes d'aide
• méthodes mobiles qui ne dépendent pas des données internes à d'autres classes
• Lorsque vous semblez avoir deux méthodes différentes qui traitent des "choses" complètes différentes, faites-en des classes séparées (c'est un effort important, spécialement sur les codes longs)
• Lorsque vous obtenez des choses séparées mais procédurales, vous pouvez commencer à appliquer des motifs qui seraient fais ton code moins répété. Jason Down a mentionné State, Strategy, Factory, Template Method et la plupart d'entre eux le feraient aussi, selon vos besoins.

Answer 5

Il a certainement besoin d'un refactoring. Quelques choses:

• Don't repeate yourself (F.E. Thread.sleep (1000); )
• diviser en quelques méthodes
• Seperate votre logique de costruction.
• Utiliser Dependency injection (Messages, isRunning doit être transmis à la méthode).
• Plutôt que d'utiliser plusieurs drapeaux utilisent Polymorphism - nice talkon this