1

myThread 是一个每秒执行的函数，基本上它读取一些必须解析和执行的数据。该函数增长了很多，它有超过 1500 行代码，如下面的示例，其中有很多 [if else if 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);
    }
}
4

5 回答 5

7

通常，当您看到复杂的条件逻辑（大量 if/else 块和嵌套条件）时，您应该考虑将代码重构为状态或策略设计模式（取决于具体情况）。看看那些以获得一些想法。

更新：
如果您在如何重构代码方面遇到问题，我最近读过一本很棒的书，它有助于分解过程（涵盖从添加版本控制到单元测试到持续集成的所有内容......还谈到了迭代地分解事情，直到你能够应用依赖注入等东西）。它没有完全深入地涵盖任何主题，因为有专门针对每个主题的整本书，但它是一个很好的起点：
.Net 中的棕地应用程序开发

于 2011-05-30T21:41:33.027 回答
3

组织此代码的一个好方法是使用“责任链”设计模式。

这个想法是为每个知道要做什么的“命令”设置一个类，例如：

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

每个命令都需要这些类之一。

于 2011-05-30T22:01:48.400 回答
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 提到了 State、Strategy，但 Factory、Template Method 和它们中的大多数也可以，这取决于您的需要。
于 2011-05-30T21:49:46.050 回答
1

它肯定需要一些重构。一些东西：

于 2011-05-30T21:57:46.733 回答
0

嗯，一开始，我认为你应该把这个庞大的代码分成几个方法。将您已评论的此逻辑部分分开。您应该为评论的每一部分使用一种方法（最低限度）。尝试开始这个：

  1. 方法： // 这里有一些解析的数据 // 如果（parsedData 匹配）做下面的事情 试着用这个方法。

  2. Method // 获取发出命令的用户

  3. Method // 基于访问级别的不同问候

  4. Method // 这里我们过滤所有以.开头的消息。这意味着它是一个命令

等等...

而且您必须制作重复的部分方法，以便您可以始终在一个地方进行更改，而不是搜索所有代码并将每个参数更改为新值。

于 2011-05-30T21:48:19.127 回答