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 ブロックとネストされた条件) を目にする場合は、コードをStateまたはStrategyデザイン パターンにリファクタリングすることを検討する必要があります (状況によって異なります)。いくつかのアイデアについては、それらをご覧ください。

更新:
コードをリファクタリングする方法に問題がある場合は、プロセスを分解するのに役立つ最近読んだ素晴らしい本があります (バージョン管理の追加から単体テスト、継続的インテグレーションまですべてをカバーしています... についても話します依存性注入などを適用できるようになるまで、物事を繰り返し分解します。) 個々の主題に特化した本全体があるため、トピックを完全に網羅しているわけではありませんが、出発点としては最適です:
Brownfield Application Development in .Net

于 2011-05-30T21:41:33.027 に答える
3

このコードを整理する良い方法は、「責任の連鎖」設計パターンを使用することです。

アイデアは、何をすべきかを知っている「コマンド」ごとに1つのクラスを持つことです。次に例を示します。

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

コードの複雑さを軽減することは、コードを分割するための最初のステップです。私が取るべき次のステップは次のとおりです。

  • さまざまなメソッドへのリファクタリング
  • 同様のメソッドを見つけ、リファクタリングし、ヘルパー メソッドを用意する
  • 内部データに依存しないメソッドを他のクラスに移動する
  • 完全に異なる「もの」を処理する2つの異なるメソッドがあると思われる場合は、それらを別々のクラスにします(これは、特に長いコードでは大きな努力です)
  • 物事が分離されているが手続き型になったら、コードの繰り返しを少なくするパターンを適用し始めるかもしれません。Jason Down は、State、Strategy について言及しましたが、Factory、Template Method と、必要に応じてそれらのほとんども同様です。
于 2011-05-30T21:49:46.050 に答える
1

間違いなくリファクタリングが必要です。いくつかのこと：

  • 繰り返さないでください(fe Thread.Sleep(1000); )
  • いくつかの方法に分割する
  • ロジックを構築から分離します。
  • 依存性注入を使用します (メッセージ、isRunning をメソッドに渡す必要があります)。
  • 複数のフラグを使用するのではなく、ポリモーフィズムを使用してください。
于 2011-05-30T21:57:46.733 に答える
0

さて、最初は、この巨大なコードをいくつかのメソッドに分割する必要があると思います。コメントしたこの論理部分を分離するため。コメントの各部分に1つのメソッドが必要です（これは最小です）。これを始めてみてください：

  1. メソッド：//ここにいくつかの解析済みデータ//（parsedDataが一致する）場合、以下のことを実行します。これの1つのメソッドを作成してみてください。

  2. メソッド//コマンドを発行したユーザーを取得します

  3. メソッド//アクセスレベルに基づいて異なる挨拶

  4. メソッド//ここでは、で始まるすべてのメッセージをフィルタリングします。これはコマンドであることを意味します

等々...

また、すべてのコードを検索してすべてのパラメーターを新しい値に変更するのではなく、常に1つの場所で変更を加えることができるように、パーツのメソッドを繰り返す必要があります。

于 2011-05-30T21:48:19.127 に答える