2011-05-30 97 views
1

myThread是一個每秒執行的函數,基本上它讀取一些必須解析和執行的數據。這個函數增長了很多,它像下面的示例一樣有1500行以上的代碼,有很多[if else else]會阻止很多重複,比如sleep或者SendToChat向控制檯發送一個命令,而且它很難維護,對其進行更改等。我如何組織這段代碼?

我想一些建議(如果可能的話,代碼示例,這將幫助我瞭解佈局),我可以如何重寫這個,我不是很有經驗,所以我不確定是否可以將此代碼轉換爲一個更好的可維護性和可讀性代碼?

也隨時評論任何功能或其他任何事情,因爲它可以幫助我改進其他錯誤的東西。

這只是代碼的一些示例,而不是整個代碼,如果您覺得您需要代碼中的任何其他信息,請隨時詢問,我會盡快發佈。

PS:這不是一個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); 
    } 
} 
+1

您可能想要在http://codereview.stackexchange.com上發佈此內容 – carlosfigueira 2011-05-30 21:40:28

+1

@carlosfigueira我認爲我的問題適合在這裏以及我不僅尋找建議,建議,但我也尋找潛在的編碼一個社區成員可能會感興趣的例子,以幫助我學習,我相信很多用戶已經通過這種體驗一次,但我會保持codereview記在腦海中我不知道它.. – Guapo 2011-05-30 21:47:29

+0

這個問題基本上是關閉的根據常見問題解答。這不是「特定的編程問題」,「每個答案都是同樣有效的」。正如carlosfigueira提到的,還有一個更多的話題stackexchange網站。 – 2011-05-30 21:52:20

回答

3

來組織這個代碼的一個好方法是使用「Chain of responsibility」設計模式。

的想法是有,知道該怎麼做,例如,「指令」一類:

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); 
    } 
} 

,將需要這些類的每一個命令之一。

+0

+1這很有趣我正在考慮做一些類似的命令,因爲我知道所有變量每個命令有但沒有確定如何去用它,這裏有幾個有趣的回覆,我將在開始之前一個接一個地進行。 – Guapo 2011-05-30 23:17:42

+0

啊...模式...好的舊模式:) +1 – JTew 2011-05-31 02:42:31

7

通常當你看到複雜的條件邏輯(大量的if/else塊和嵌套條件的),你應該考慮重構你的代碼到StateStrategy設計模式(視情況而定)。看看那些想法。

UPDATE:
如果您有任何關於如何重構你的代碼的麻煩,有一個偉大的書我最近讀到,有助於打破下跌過程(從加入版本控制涵蓋一切,單元測試,持續集成......還談論瞭如何反覆分解東西,直到你能夠應用諸如依賴注入等東西)。它不包括任何的完整深度的主題,因爲有專門爲每個個體對象整本書,但它是一個很好的起點:
Brownfield Application Development in .Net

+0

+1非常感謝,因爲某些原因,我特意爲這些例子提供了很多幫助。當我看到一些代碼和材料等時,我有更多的設施可以學習。 – Guapo 2011-05-30 21:48:46

0

嗯,開始的時候,我想你應該拆分這巨大的代碼分成幾種方法。分開您評論的這些邏輯部分。對於評論的每個部分你都應該有一個方法(這是最低限度的)。最佳食用量開始這樣的:

  1. 方法://一些解析數據HERE //如果(parsedData配襯)做下面的東西 嘗試讓這一個方法。

  2. 方法//獲取發出命令

  3. 方法用戶//根據接取水平不同的問候語

  4. 方法//在這裏,我們篩選以a開頭的所有消息。它是一個命令

...等,即重複這樣你就可以做出改變總是在一個地方部分的

你必須做的方法,這意味着,不搜索throught所有代碼和將每個參數更改爲新值。

1

我通常會做一個小竅門,雖然不是一種好的重構方法,但它使代碼更具可讀性。

而是具有:

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

...我用:

if (condition1) { 
    statement1; 
    return; 
} 

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

降低了代碼的複雜性是第一步,除了打破它。爲了讓我把接下來的步驟是:

  • 重構到不同的方法
  • 發現類似的方法,重構和其輔助方法
  • 移動不依賴內部數據的其他類方法
  • 當你似乎有兩種不同的方法來處理完全不同的「事物」時,將它們分開分類(這是一個主要的努力,特別是對冗長的代碼)
  • 當你把事情分開但程序化時,你可以開始應用模式,製作你的代碼少重複。 Jason Down提到了狀態,策略,但工廠,模板方法,其中大部分也會根據您的需求來做。
+0

謝謝這些都是很好的建議,因爲我有點失落如何開始定義在哪裏放什麼等...分離所有的條件將幫助我更容易地看到通過 – Guapo 2011-05-30 23:23:20

1

它肯定需要一些重構。有幾件事情:

+0

+1確實最初的代碼是非常小的代碼我沒有看到它像它那樣的問題,但它開始增長太快,有很多命令,我厭倦了所有的混淆塊依賴注入似乎正是我需要我仍然有一些難以把它在實踐中認爲 – Guapo 2011-05-30 23:20:27