FxCop、あるいはVisualStudio付属のコード分析で、厳しめの規則セットを使うと、以下のようなコードに対して警告が出る。
public class Person { public string[] Nicknames { get; private set; } ... }
CA1819 コレクションを返すために、またはメソッドに設定するために、'Person.Nicknames' を変更してください。
日本語がおかしい。
配列でなくコレクションを返すか、プロパティでなくてメソッドにしろという意味だ。
MSDNにはもう少し詳しく書いてるんだけど、にしてもこれ分かりにくいんだよな。
プロパティが読み取り専用であっても、プロパティで返される配列は書き込みから保護されません。 配列の改ざんを防ぐには、プロパティで配列のコピーを返す必要があります。 一般に、このようなプロパティを呼び出すときのパフォーマンス低下は理解されません。 具体的には、インデックス付きプロパティとして、プロパティを使用する場合があります。
プロパティとして配列を公開しちゃうと、上のサンプルコードのようにsetterをprivateにしてたとしても、
//Person person; person.Nicknames[0] = "hoge";
というふうに外から要素を変更することができちゃいますよ? というのが1文目の意味。
で、配列のプロパティのまま、この外部からの変更を防ごうとしたら、配列そのものでなくてそのコピーを返すようにしないといけない。
以下のようなイメージだ。
private string[] _nicknames; public string[] Nicknames { get { return (string[])_nicknames.Clone(); } private set { _nicknames = value; } }
これが2文目。
だけど、こんなふうに実装しちゃうと、外からはまさか実装がこんなふうになってると思わんから、気軽にプロパティを参照しちゃうと、そのたびに配列のコピーが作られてパフォーマンスに影響するよ、というのが3文目の意味だ。
たとえば下のように書いたら配列のコピーが3回も作られる。
//Person person; if (0 < person.Nicknames.Length) { return person.Nicknames[person.Nicknames.Length - 1]; }
なので、気軽に何度もアクセスさせないように、プロパティじゃなくて GetNicknames() みたいなメソッドにしましょうというのが提案されてる対処方法のひとつなんだけど、それってちょっと微妙じゃない?と思う。
プロパティにすれば内部実装を意識せずに使えるよというそもそもの思想と逆行してる気もするので。
(というかCloneNicknames()みたいな名前にしろということなのかな…)
というわけで、もうひとつの対処案、配列でなくてコレクションを公開するというのをオススメします。
そこで便利なのが、.NET Framework 4.5から追加された、IReadOnlyList
配列もこのインタフェースを持っているので、実体は配列のまま読み取り専用コレクションとして公開することができる。
public class Person { private string[] nicknamesArray; public IReadOnlyList<string> Nicknames { get { return nicknamesArray; } } ... }
//Person person; person.Nicknames[0] = "hoge"; // ← これはエラーになる
IEnumerable
インデクサも使えないしね。