Refactoring:10分でコーディング

前回の続きです。
頂いたコメントを参考に更なるリファクタリングを行いました。

class Cards
{
    public string[] Deal(int numPlayers, string deck)
    {
        int needCount = deck.Length - (deck.Length % numPlayers);
        string[] hands = Enumerable.Repeat(string.Empty, numPlayers).ToArray();

        for (int cardIndex = 0; cardIndex < needCount; cardIndex++)
        {
            int playerIndex = cardIndex % numPlayers;
            hands[playerIndex] += deck[cardIndex];
        }

        return hands;
    }
}
かなりすっきりしました。
前回のリファクタリングで導入した yield return による順序のオブジェクト化は半分ネタだったのでともかくとしても、TrimDeck メソッドは今にして思うと過剰なリファクタリングだった気がします。(でも、順序のオブジェクト化に伴い for 文を foreach 文にしたくって TrimDeck という形を取った、というのもあるんですよ^^;)
CreateInitialHands メソッドも、Enumerable.Repeat の導入によって不要となりましたね。
ちなみに、メソッドの中身が一行だとしても、場合によってはメソッド化する意味があります。処理に名前を付けることで可読性の向上が期待できる場合があるからです。しかし、今回の場合は、メソッド化せずともローカル変数の名前で処理の目的を示すことができますので、インライン化を行いました。
needCount については、前回のようにまず needlessCount を求めてから needCount を求めるようにする方法と、今回のように一行にする方法と、どちらの方が良かったのか悩み所でしたが…。

最後に for 文に関してですが、前回の記事のコメント欄では hands[i % numPlayers] += deck[i]; と書きましたが、そうはせずにまず playerIndex を求めるようにしました。これは、i % numPlayers が何を求めているのかわかりづらいかなと思ってのことです。前述の通り、ローカル変数の名前によって可読性が向上したのではないかと思います。

ということで、かなりいい感じになったなぁと思いますがどうでしょ？