Как я могу организовать этот код? - PullRequest
1 голос
/ 31 мая 2011

myThread - это функция, которая выполняется каждую секунду, в основном она считывает некоторые данные, которые необходимо проанализировать и выполнить. Эта функция значительно выросла, и она содержит более 1500 строк кода, как в примере ниже, с большим количеством [если еще, если еще] блокирует множество повторений, таких как 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);
    }
}

Ответы [ 5 ]

7 голосов
/ 31 мая 2011

Часто, когда вы видите сложную условную логику (множество блоков if / else и вложенные условия), вы должны рассмотреть возможность рефакторинга вашего кода в шаблоны проектирования State или Strategy (зависит отситуация).Посмотрите на них для некоторых идей.

ОБНОВЛЕНИЕ:
Если у вас возникли проблемы с реорганизацией кода, я недавно прочитал отличную книгу, которая помогает разбить процесс (охватывает всеот добавления контроля версий, до модульного тестирования, до непрерывной интеграции ... также говорится об итеративном разделении, пока вы не сможете применять такие вещи, как внедрение зависимостей и т. д.).Он не охватывает ни одну из тем в полной мере, потому что есть целые книги, посвященные каждому отдельному предмету, но это отличная отправная точка:
Разработка приложений Brownfield в .Net

3 голосов
/ 31 мая 2011

Хороший способ организовать этот код - использовать шаблон проектирования " Цепочка ответственности ".

Идея состоит в том, чтобы иметь один класс для каждой «команды», которая знает, что делать, например:

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

Вам понадобится один из этих классов для каждой команды.

1 голос
/ 31 мая 2011

Это определенно требует некоторого рефакторинга. Несколько вещей:

1 голос
/ 31 мая 2011

Я обычно делаю небольшой трюк, который, хотя и не является хорошим методом рефакторинга, делает код более читабельным.

Вместо того, чтобы:

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

... Я использую:

if (condition1) {
    statement1;
    return;
}

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

Уменьшение сложности кода - это первый шаг к его разделению.Следующие шаги, которые я должен предпринять:

  • рефакторинг на разные методы
  • поиск похожих методов, рефакторинг и наличие вспомогательных методов
  • методы перемещения, которые не зависятвнутренних данных в другие классы
  • когда у вас есть два разных метода, которые имеют дело с совершенно разными "вещами", делайте их отдельными классами (это серьезное усилие, особенно в отношении длинных кодов)
  • когда вы разделяете вещи, но процедурные, вы можете начать применять шаблоны, которые сделают ваш код менее повторяющимся.Джейсон Даун упомянул «Состояние», «Стратегия», «Фабрика», «Шаблонный метод» и большинство из них также подойдет, в зависимости от ваших потребностей.
0 голосов
/ 31 мая 2011

Ну, для начала, я думаю, вы должны разделить этот огромный код на несколько методов.Чтобы отделить это логические части, которые вы прокомментировали.У вас должен быть один метод для каждой части комментария (это минимум).Попробуйте для начала это:

  1. Метод: // НЕКОТОРЫЕ ПАРСИРОВАННЫЕ ДАННЫЕ ЗДЕСЬ // если (parsedData соответствует) сделать следующие вещи, попробуйте сделать один метод этого.

  2. Метод // Получить пользователя, который выполнил команду

  3. Метод // различные приветствия в зависимости от уровня доступа

  4. Метод// Здесь мы фильтруем все сообщения, которые начинаются с.это означает, что это команда

и так далее ...

И вы ДОЛЖНЫ создавать методы повторяющихся частей, чтобы вы могли вносить изменения всегда только в одном месте,не выполнять поиск по всему коду и изменять каждый параметр на новое значение.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...