ラムダ式を利用したリファクタリングの例 その４ / 前編

デジカメとかで撮影した画像ファイルの、Exifタグの中身を参照する、こんなコードがありました。コンソールアプリで第一引数にファイルパスをとる形です、Mainメソッドのみ書き出します。

private static void Main(string[] args)
{
  if (args.Length < 1)
  {
    Console.WriteLine("Less Arguments.");
    return;
  }
  try
  {
    using (Bitmap bmp = new Bitmap(args[0]))
    {
      foreach (PropertyItem pr in bmp.PropertyItems)
      {
        Console.Write("ID=0x{0:X}({1}), Len={2}, Type={3}({4})",
          pr.Id,
          PropertyItemReader.GetExifTagId(pr.Id),
          pr.Len,
          pr.Type,
          PropertyItemReader.GetExifType(pr.Type));

        if (pr.Len > 0)
        {
          if (pr.Type == 2)
          {
            Console.WriteLine(", Value={0}", 
                              PropertyItemReader.ReadStringValue(pr));
          }
          else if (pr.Type == 3 || pr.Type == 4 || pr.Type == 5 ||
                   pr.Type == 9 || pr.Type == 10)
          {
            StringBuilder sb = new StringBuilder();
            foreach (object o in PropertyItemReader.ReadValues(pr))
            {
              sb.Append(o.ToString());
              sb.Append(",");
            }
            sb.Remove(sb.Length - 1, 1);
            Console.WriteLine(", Value={0}", sb.ToString());
          }
          else
          {
            Console.WriteLine("");
          }
        }
        else
        {
          Console.WriteLine("");
        }
      }
    }
  }
  catch (Exception e)
  {
    Console.WriteLine(e.Message);
  }
}

ちなみに、実行するとこんな結果を吐き出します。

この中の「PropertyItemReader」というのは、別途作ったstaticなクラスで、PropertyItemからExif-Tagが文字列ならその値を文字列で取り出したり、数値型の配列ならArrayで取り出したり。または、Tag-IDやTag-Typeを表示名に変換したりといった機能を持ちます。(とりあえずここでは、そいつのコードは割愛します。)

で、コードをぼーっと眺めてみて、なんだかかっこ悪い。if文があまり意味もなくネストしてるし、配列をカンマ区切りの文字列にするために、StringBuilderを使っているけど、string.Joinを使えばも少しシンプルになりそう。それと、数値型の判断をしている条件文がなんだか冗長。

と、気がついたところを直してみました。こうなりました。なお、これ以降説明用に、引数チェックと例外処理は省略します。

private static void Main(string[] args)
{
  using (Bitmap bmp = new Bitmap(args[0]))
  {
    foreach (PropertyItem pr in bmp.PropertyItems)
    {
      Console.Write("ID=0x{0:X}({1}), Len={2}, Type={3}({4})",
        pr.Id,
        PropertyItemReader.GetExifTagId(pr.Id),
        pr.Len,
        pr.Type,
        PropertyItemReader.GetExifType(pr.Type));

      if (pr.Len > 0 && pr.Type == 2)
      {
        Console.WriteLine(", Value={0}", 
                      PropertyItemReader.ReadStringValue(pr));
      }
      else if (pr.Len > 0 && new short[] {3,4,5,9,10}.Contains(pr.Type))
      {
        Console.WriteLine(", Value={0}", string.Join(",",
                      PropertyItemReader.ReadValues(pr).Cast<object>()));
      }
      else
      {
        Console.WriteLine("");
      }
    }
  }
}

だいぶいい感じになったかな。でも、なんだかまだちょっと不満。何でだろう。

どうやら、if ～ else if ～ elseのブロックが、それぞれがシンプルな処理なだけに、どうも気に入らない。ような気がしてきた。こいつをどうにかすることを考えよう。

が、今日はここまで。寝ないとまずいってば。続きは次のエントリで。