NEGAKULIFE

ネ学生のQoLを上げるブログ

ネ学生のQoLを上げるブログ

課題が終わらないのは仕様

SD攻略決死隊 - 水泡と化す過去の軌跡 後編

こんにちは、夕飯を食べ終わりました。
それにしても今週忙しすぎないですか?

前編では、チャットクライアントとかそういう話しました。後編はプログラミングの話です。 「途中で動かなくなった」とか言っている人向けかもしれないです。

ソースコードがきたねえ

僕も余り人のことは言えないんですが、これに尽きます。「色々課題にそって追加してったら動かなくなった」っていう人のソースコードを診ると、ゴチャゴチャでよくわからないことになっています。C言語でも書いてるのか????ってツッコミたくなります。そこまでは行かずとも「関数・メソッド」という括りを上手く使いこなせれば、もっとより良いコードが書けるのではないかと思います。(あくまで個人的感想で、ここは結構ナイーブな問題なので宗派が違うひとはすいません)

例えば、以下のような、サーバーサイドのプログラムがあったとします。

@Override
public void run() {
    try {
        out = new PrintWriter(socket.getOutputStream(), true);
        in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
        String fromClient;
        while((fromClient = in.readLine()) != null) {
            System.out.println(fromClient);
            if(text.startsWith("user")) {
                String[] args = text.split(" ");
                if(args.length == 4 && args[2].equals("pass")) {
                    if(checkId(args[1], args[3])) {
                        //ログインしたらそのクライアントを通す
                        out.println("0 login succeed");
                        userName = args[1];
                        //ユーザーがログインしたことを全員に知らせる
                        for(ChatMThread client: member) {
                            client.out.println("login" + userName);
                        }
                    } else {
                        out.println("100 password invalid");
                        end();
                    }
                }
            } else if(text.startsWith("chat")) {
                //あといろいろ処理
            }
        }
    } catch(IOException e) {
        end();
    }
}

読めはする

なんとなく意味はわかります。サーバー側のin(前回参照)から受け取った文字列をスペースで分割して、一つ目が「login」だったらログインをする処理です。一見良いように見えますが、これに処理を加えていってしまうと、どんどんコードが肥大化してしまいます。あなたのコードもそうなっていませんか?

改善していく

プログラミングは「人類が楽をするため」に存在します(えしら談)。何度も同じことを書くような「めんどくさい」ことをするのって本末転倒じゃないですか?

また、同じ処理を色んな所に何回も書くのは、効率が悪いだけではなく、メンテナンスがしづらくなります。「後から見返してよくわからない」「一発で自分の書いた処理がどこにあるのかがわからない」のはこれが原因です。

細かくちぎっては投げ

なので、処理を全部わけちゃいましょう。

最初に目をつけるのは、クライアントにメッセージを投げる部分です。「out.println」とか「client.out.println」とか、上のログイン処理だけでも3回も出てきてますよね?これを全部分けてしまいます。しかし一様に「クライアントにメッセージを投げる」といっても、二種類存在する事にお気づきでしょうか。

「一つのクライアントにメッセージを投げる」のと「全てのクライアントにメッセージを投げる」の二種類があります。これを前提にこんな改善を行ってみます。

@Override
public void run() {
    try {
        out = new PrintWriter(socket.getOutputStream(), true);
        in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
        String fromClient;
        while((fromClient = in.readLine()) != null) {
            System.out.println(fromClient);
            if(text.startsWith("user")) {
                String[] args = text.split(" ");
                if(args.length == 4 && args[2].equals("pass")) {
                    if(checkId(args[1], args[3])) {
                        //ログインしたらそのクライアントを通す
                        send("0 login succeed");
                        userName = args[1];
                        //ユーザーがログインしたことを全員に知らせる
                        sendAll("login" + userName);
                    } else {
                        send("100 password invalid");
                        end();
                    }
                }
            } else if(text.startsWith("chat")) {
                //あといろいろ処理
            }
        }
    } catch(IOException e) {
        end();
    }
}

public void send(String msg) {
    out.println(msg);
}

public void sendAll(String msg) {
    for(ChatMThread client: member) {
        client.out.println(msg);
    }
}

送る部分を分けてみました。結果的なコードは長くなっていますが、「どこで何をやっているのか」が明確になりました。ここで、更に一工夫加えてみましょう。sendAllは「全てのクライアントにメッセージを投げる」ものですが、中身を見ると「全てのクライアントに順番に見て、一つのクライアントにメッセージを投げる」ということをしています。これを考えると、以下のように書き換えることも出来ます。

public void send(String msg) {
    out.println(msg);
}

public void sendAll(String msg) {
    for(ChatMThread client: member) {
        client.send(msg);
    }
}

こうすると、全ての「メッセージを投げる」処理が最終的に「sendメソッド」に集約されることになります。これには様々なメリットがあると思います。

例えば、何かの都合で(まぁこの課題ではありえませんが)、「クライアントに送る方法がout.println()からout.message()に変わったから!」と仕様が変わったとします。そういう場合に最初のコードだとどうでしょう。三箇所も書き換えなければいけません。処理が増えていた場合は、もっと多くの箇所を修正する必要があります。次のコードだと、二箇所の変更で済みます。最後のコードだと、一箇所で済みます。これって凄い大きいことじゃないですか?!

分割するセンス

かといって、正直いってどこまで分割していくかというのは好みの問題です。「細かすぎるとみづれえ死ね!」っていう人もいれば「細ければ細かいほど良い(世の中にはメソッドは1インデントで納めろという人もいるとか…)」という人もいます。

結果的に動けばどうでもいいです。ただ自分の書いたコードを後から見返して苦痛なのは嫌だよね!

まとめ

  • 今回のSDは爆死者続出
  • ほんとにソース理解できてる?
  • スパゲッティコードは殺せ
  • 分割はセンス
  • ちなみに僕は「あ、この処理さっき見たな」ってなったら分割してます
  • 「作り直しの美学」
  • 猫はすくすく育ってるよ