レビューをした後はのどの調子がよくなります。普段あんまり喋らないからね。
今回の対象プロジェクト
- C#, C++ の混在プロジェクト。
- 組み込み機器を制御する GUI アプリで、 GUI を WPF 、ファームウェアを C++ で開発。
- レビュイーは業務経験1～3年目くらいの子たちです。
だいたい前回と同じです。
[C#] List の Count と Count() の違いを逆コンパイルから確認してみる
var list = new List<int>() { 1, 2, 3 };
Console.WriteLine("Count : {0}", list.Count);
Console.WriteLine("Count() : {0}", list.Count());
この例ではどちらも 3 が表示されます。
-
Countは「プロパティ」で、 List 自体の一部です。誤差レベルですが、こちらの方がオーバーヘッドが少ないです。 -
Count()は LINQ による「拡張メソッド」です。List 以外でも Array や Dictionary など IEnumerable を継承しているクラスに対して使えます。
この 2 つで迷ったらプロパティの方を使いましょう。
さて、それぞれ中身がどうなっているのかは、次のようにすることで確認できます。
- VisualStudioのメニューの [ツール] > [オプション] > [テキストエディター] > [C#] > [詳細] > [逆コンパイルされたソースへのナビゲーションを有効にする] を ON にする。
- コードエディタ上で当該のプロパティやメソッドを右クリック > [定義へ移動] する。
.NET 6 では、 Count の定義はこんな↓感じでした。
public int Count => _size;
Count() の定義はこんな↓感じでした。
public static int Count<TSource>(this IEnumerable<TSource> source)
{
if (source == null)
{
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
}
if (source is ICollection<TSource> collectionoft)
{
return collectionoft.Count;
}
if (source is IIListProvider<TSource> listProv)
{
return listProv.GetCount(onlyIfCheap: false);
}
if (source is ICollection collection)
{
return collection.Count;
}
int count = 0;
using (IEnumerator<TSource> e = source.GetEnumerator())
{
checked
{
while (e.MoveNext())
{
count++;
}
}
}
return count;
}
List は ICollection を継承していますので、結局は Count プロパティが呼び出されることになります。
Count() の方が型のチェックが入る分、わずかにオーバーヘッドが大きいと言えそうです。
[C#] enum から文字列への変換に Dictionary を使う必要はないかも
こんなコードがプルリクに上がりました。
enum MyEnum
{
Value1,
Value2,
Value3
}
// enum 値を文字列に変換する
string ConvertToString(MyEnum value)
{
var dict = new Dictionary<MyEnum, string>() {
{ MyEnum.Value1, "Text1" },
{ MyEnum.Value1, "Text2" },
{ MyEnum.Value1, "Text3" },
};
string? ret = null;
if (!dict.TryGetValue(value, out ret))
{
return "";
}
return ret;
}
これは次のように、C# の「switch 式」を使って直したほうがよさそうです。
string ConvertToString(MyEnum value)
{
return value switch
{
MyEnum.Value1 => "Text1",
MyEnum.Value2 => "Text2",
MyEnum.Value3 => "Text3",
_ => throw new ArgumentOutOfRangeException()
};
}
最初の ConvertToString() は、呼ばれるたびに同様の Dictionary のインスタンスが作られるため非常に無駄です。
人間から見たら一瞬ですが、 new はそれだけでもそれなりのオーバーヘッドが乗ります。
もしこれが大量のデータを GUI に表示したり CSV ファイルへエクスポートするのに使われるとメモリフラグメンテーションの原因にもなりやすく、さらにパフォーマンスが落ちるかもしれません。
また Dictionary の内部アルゴリズムは ハッシュテーブル ということですが、値の検索にはハッシュ値の計算や比較、ハッシュ衝突時のケアなど諸々の処理が動いており、O(1) に近いとは言うものの、単に switch-case を使うよりははるかにたくさんの処理が動いています。
ちなみに、キーの数が数個程度であれば、Dictionary.TryGetValue よりも List.Find で線形探索するほうが高速です。
簡単にですが、私の手元 (CPU i9-11900 @ 2.50GHz, RAM 32768MB) で測った結果を載せておきます。
| 項目 | 時間 |
|---|---|
| Dictionaly (キー3件) の new １回あたり | 8us |
| Dictionaly.TryGetValue １回あたり | 3us |
| switch 式 １回あたり | 0.003us |
| List.Find (キー3件) １回あたり | 0.6us |
[プログラミング全般] コメントを書こう
ソースコードにコメントが全くないプルリクをレビューすることもちらほらあります。だいたい次のような傾向かなと思います。
- 「コメントを書く必要がないくらいコードをわかりやすくしよう！」という信念を持っている
- コメントに何を書けばよいかわからない（ので、結果的に何も書かない）
コメントの主な目的は保守性の向上です。仕事で書くコードを保守するのは自分だけとは限りません。「なぜ？（そのように実装した意図）」はコメントでなければ表現が困難です。他の誰かがこのコードを変更するときに、頭に入れておいてほしい注意事項とも言えます。
こういうことはプルリクに書くべきというのもありますが、プログラマが最も目にするのはソースコードです。git を使っていれば blame でプルリクまで辿れますが、常にそこまで気を回せるでしょうか？
- 解が他にもあるのに、なぜそれを選択したのか？
- 工数・工期や技術的な都合で見送ったリスクはあるのか？
- 別モジュールの不具合を回避するための一時的な処理か？
- 参考にしたWebページや論文はあるのか？
プルリクや設計書に重要なことが書いてあるなら、そこへ誘導することもまた、コメントの役割でしょう。
そもそも何を書くべきか（何を書くのはダメなのか）がわからなければ、 ソースコード コメント 書き方 とかで検索してみましょう！
機械的なトレーサビリティのためにコメントを使わないように注意です。 全ての変更箇所に、変更履歴や要求ID(Issue番号など) を残す必要はありません。そういったものは git などを使いってコミットメッセージに書き、必要な時は blame で探しましょう。
ソースコードに残すのは、トレーサビリティというより別の誰か（あるいは将来の自分）にコードを説明するための情報です。その情報として必要であれば、要求IDや資料へのリンクを残します。それが結果としてトレーサビリティ情報に見えることがありますが、その目的は違います。
おわりに
なんで new や検索の速度に見積りを立てられるのか不思議に思われました。
私は少し前にゲームエンジンや標準ライブラリのクローンの開発に躍起になっていたので、
C# のコードが CPU に流れるまでに何が起こるのか、なんとなくわかるようになっちゃってるためかと思います。
でも後輩にそこまでの理解を求めるのかというと、今はそのフェーズではないような気もします。年々複雑化する開発環境についていくのに精一杯だろうかと。それよりはまず開発に慣れてほしいかなと。
しかし多分こういう細かい積み重ねが「おたくのアプリなんか重いね」っていうのに響いてくるのかと思うと本当に、本当に悩ましいです。特に弊社、大きなデータを扱うアプリを作ることが多いものでして。
ところで当然ではありますが、私たちが普段使っている標準ライブラリ (List とか) も誰かが書いたソースコードがあります。たまには今回紹介した方法でソースコードを覗いてみると、効率の良い書き方や関数名の付け方など新しい学びが得られるかもしれません。
ただしこれらのコードも何かしらのライセンスで守られていますので、あくまで勉強のために参照し、コピペして使うのは避けましょう。