保守しやすいコードの反面教師​ （アンチパターン） その2

保守しやすいコードの反面教師 （アンチパターン） その2 2022/3/18 須藤

自己紹介  ID：suusanex（ connpass・Twitter・GitHub共通）  名前：須藤圭太  サイエンスパーク株式会社という独立系ソフトウェアベンダーに所属  4年ほど受託開発で、上流から下流まで全部を回す  ここ6年ほどは、自社製品開発を担当  Windowsアプリ開発のネタが多い

概要  実際の開発で見たアンチパターンをいくつか紹介して、これ以上無駄な苦労を 再生産しないようにしていきたい  今回は設計レベルの話ではなく、C#のコーディングのポイントを中心に

目次  nullの可能性を無視してアクセスする  異常系なのに例外をcatchして無視する  LINQ向きなのに頑張ってループ処理  UIのメソッドを同期で書いて応答無し  時間をintで表す  パス判定を==

nullの可能性を無視してアクセスする  何を見たか    RegistryKey.OpenSubKey(“・・・”).GetValue(“・・・”)  サブキーが無い場合OpenSubKeyがnullを返すので、NullReferenceException 何が問題か   nullを返しうるメソッドの戻り値を、そのまま次の処理で使用 Null参照の例外を出してしまうと、発生箇所の情報がなく解析が辛い どう改善するか  nullチェックして自分で例外を投げるか、?や??を上手く利用する  if(value == null) throw new Exception(“処理Aがnullを返した”)  var value = RegistryKey.OpenSubKey(“・・・”)?.GetValue(“・・・”) ?? throw new Exception(“KeyかValueが無かった”);

異常系なのに例外をcatchして無視する    bool func(){ try{ … 何を見たか } catch(Exception){  各関数の中で例外をcatch、voidで終了もしくはreturn false return false;  成功扱いなのかと思ったら、明らかに異常系に入るべき処理 } 何が問題か  例外を捨てるのは、「その例外は正常系だ」と判断できる関数がやるべき  例外発生の情報とその後のエラー処理の情報が分かれるため、解析しづらい どう改善するか   「その例外は正常系だ」と判断できる場合を除き、例外はそのままthrowする 最後まで処理されなかった例外の処理方針は決めておき、スレッドの入り口やその ための例外ハンドラで処理する

LINQ向きなのに頑張ってループ処理   var found = new List<string>(); foreach(var val in values){ 何を見たか if(val.StartsWith(“a”){ found.Add(val);  LINQにぴったりのメソッドがある処理を、 頑張ってループで書く } } 何が問題か   何でもLINQ最高とは言わないが、容易に置き換えられるなら可読性も書きやすさも 優秀なLINQを使いたい どう改善するか   var found = values.Where(d => d.StartsWith(“a”)); このfoundはそのままforeach等に使えるので、最後に必要になった時にToList()や ToArray()すれば良い（IEnumearable型）

UIのメソッドを同期で書いて応答無し  何を見たか   何が問題か    「ボタンを押したら、処理中は入力を禁止する」の実現方法が、単に同期処理をし ているだけなので、応答無しになっている 10年前ならともかく、今のUIで応答無しはほぼバグ扱い ユーザーがせっかちだと、強制終了されかねない どう改善するか  async/awaitという便利すぎる記法が入って同期メソッドとほとんど変わらなく なったので、使おう  入力を禁止するなら、DockPanel等のパネルを丸ごとIsEnabled=falseにすれば良い

時間をintで表す  何を見たか   （intervalは時間で、単位はミリ秒） 何が問題か    void func(int interval) 単位がぱっと見で分かりづらく、ミスりやすい 変数名にMillisecondと付けてもいいが、無駄に長い どう改善するか  .NETにはTimeSpanという型があるので、それを使う  ミリ秒が欲しい場合もTimeSpan.TotalMillisecondsで取れる  マジックナンバーより名前付き変数、型無しよりも型有り、が保守性を上げる

パス判定を==  何を見たか   何が問題か   ファイルパスの一致判定で、file1 == file2 Windowsのファイルパスは、大文字小文字の違いを無視する どう改善するか  file.Equals(file2, StringComparison.InvariantCultureIgnoreCase)  他にも、folder1 + file2等も良くない。  パスの結合はPath.Combine()を使うなど、パスをミスなく処理できるメソッドは多 いので、演算子で処理しない

まとめ  この内容を「常識だろ」と思える人は問題なし  思ったより、このアンチパターンに引っかかってしまう人は多い  このレベルは知識の共有で抜け出して、もっと高レベルなことで時間を使おう