C#と諸々

C#がメインで他もまぁ諸々なブログです
おかしなこと書いてたら指摘してくれると嬉しいです(´・∀・`)
つーかコメント欲しい(´・ω・`)

--/--/-- --:--
上記の広告は1ヶ月以上更新のないブログに表示されています。
新しい記事を書く事で広告が消せます。
タグ:
トラックバック(-) | コメント(-) | このエントリーを含むはてなブックマーク
2009/06/13 00:28
前回の続きです。
頂いたコメントを参考に更なるリファクタリングを行いました。

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 が何を求めているのかわかりづらいかなと思ってのことです。前述の通り、ローカル変数の名前によって可読性が向上したのではないかと思います。

ということで、かなりいい感じになったなぁと思いますがどうでしょ?
タグ: .NET C#











トラックバックURL↓
http://csharper.blog57.fc2.com/tb.php/268-acec6c2c

上記広告は1ヶ月以上更新のないブログに表示されています。新しい記事を書くことで広告を消せます。