2013年4月4日木曜日

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

前編の続き。Exifファイルのタグ一覧をコンソール上に表示するプログラム。前編最後のコードはこうでした。
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 の構文が条件も各々の処理も「PropertyItem」をもとにしていることに気がつき、せめてもう少し見栄えが良くなるように、条件と処理を管理するこんなシンプルなクラスを作ってみました。

パラメータに対する条件と処理のペアを複数個登録しておき、実行時には最初に条件に合致した処理を実行する。というクラスです。switch ~ case や if ~ else if ~ else をオブジェクトにしたような存在。
public class SwitchAction<T>
{
  private List<Tuple<Func<T, bool>, Action<T>>> switches;
  private Action<T> actionDefault = _ => { };

  public SwitchAction()
  {
    switches = new List<Tuple<Func<T, bool>, Action<T>>>();
  }

  public SwitchAction(Action<T> actionDefault) : this()
  {
    this.actionDefault = actionDefault;
  }

  public void AddCase(Func<T, bool> condition, Action<T> action)
  {
    switches.Add(Tuple.Create(condition, action));
  }

  public void Action(T target)
  {
    var sw = switches.FirstOrDefault(s => s.Item1(target));
    if (sw != null)
      sw.Item2(target);
    else
      actionDefault(target);
  }
}

このクラスを使うと、こう書きかえることができます。
private static void Main(string[] args)
{
  using (Bitmap bmp = new Bitmap(args[0]))
  {
    var sw = new SwitchAction<PropertyItem>(_ => Console.WriteLine());

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

    foreach (PropertyItem property in bmp.PropertyItems)
    {
      Console.Write("ID=0x{0:X}({1}), Len={2}, Type={3}({4})",
        property.Id,
        PropertyItemReader.GetExifTagId(property.Id),
        property.Len,
        property.Type,
        PropertyItemReader.GetExifType(property.Type));

      sw.Action(property);
    }
  }
}

んん~?な~んか違うなぁ。どの条件の時も処理はすべて「Console.WriteLine」だ。だから、コンソールに出力する文字列だけを取り出す処理にするように変えよう。なので、条件と処理を管理するクラスを、さらに戻り値を返す物を別に作ろう。
public class SwitchFunc<T, TResult>
{
  private List<Tuple<Func<T, bool>, Func<T, TResult>>> switches;
  private Func<T, TResult> funcDefault = _ => default(TResult);

  public SwitchFunc()
  {
    switches = new List<Tuple<Func<T, bool>, Func<T, TResult>>>();
  }

  public SwitchFunc(TResult resultDefault) : this()
  {
    this.funcDefault = _ => resultDefault;
  }

  public SwitchFunc(Func<T, TResult> funcDefault) : this()
  {
    this.funcDefault = funcDefault;
  }

  public void AddCase(Func<T, bool> condition, Func<T, TResult> func)
  {
    switches.Add(Tuple.Create(condition, func));
  }

  public TResult Func(T target)
  {
    var sw = switches.FirstOrDefault(s => s.Item1(target));
    if (sw != null)
      return sw.Item2(target);
    else
      return funcDefault(target);
  }
}

これを使うと、foreach ループの中身が一文であらわされます。なので、Array.ForEachを使う形に変えてみました。こうなりました。ブロックがusing の一つだけになりました。
private static void Main(string[] args)
{
  using (Bitmap bmp = new Bitmap(args[0]))
  {
    var sw = new SwitchFunc<PropertyItem, string>(string.Empty);

    sw.AddCase(p => p.Len > 0 && p.Type == 2,
      p => ", Value=" +
        PropertyItemReader.ReadStringValue(p));
    sw.AddCase(p => p.Len > 0 && new short[] {3,4,5,9,10}.Contains(p.Type),
      p => ", Value=" +
        string.Join(",", PropertyItemReader.ReadValues(p).Cast<object>()));

    Array.ForEach(bmp.PropertyItems, p =>
      Console.WriteLine("ID=0x{0:X}({1}), Len={2}, Type={3}({4}){5}",
        p.Id,
        PropertyItemReader.GetExifTagId(p.Id),
        p.Len,
        p.Type,
        PropertyItemReader.GetExifType(p.Type),
        sw.Func(p)));
  }
}

…悪くはないけど、わざわざ別にクラスを作ったメリットが感じられないコードになった…。気がする。これなら三項演算子を重ねてみても変わらないんじゃないかなぁ?

ってことで書き変えてみたのがこれ。
private static void Main(string[] args)
{
  using (Bitmap bmp = new Bitmap(args[0]))
  {
    Array.ForEach(bmp.PropertyItems, p =>
      Console.WriteLine("ID=0x{0:X}({1}), Len={2}, Type={3}({4}){5}",
        p.Id,
        PropertyItemReader.GetExifTagId(p.Id),
        p.Len,
        p.Type,
        PropertyItemReader.GetExifType(p.Type),
        p.Len > 0 && p.Type == 2 ? 
            ", Value=" + PropertyItemReader.ReadStringValue(p) :
          p.Len > 0 && new short[] {3,4,5,9,10}.Contains(p.Type) ?
            ", Value=" + string.Join(",", 
              PropertyItemReader.ReadValues(p).Cast<object>()) : ""));
  }
}

あー。動くけど、もはや何がしたいのか誰にもわからんコードになったなぁ。三項演算子を使うにしても、せめて「WriteLine」の{5}に当たる処理は、インラインにするには複雑すぎるので、外に追い出そう。ということで、引数チェックや例外処理を含む、最終的なコードはこうなりました。
private static void Main(string[] args)
{
  if (args.Length < 1)
  {
    Console.WriteLine("Less Arguments.");
    return;
  }
  try
  {
    using (Bitmap bmp = new Bitmap(args[0]))
    {
      Func<PropertyItem, string> getvalues = p =>
        p.Len > 0 && p.Type == 2 ?
          ", Value=" + PropertyItemReader.ReadStringValue(p) :
        p.Len > 0 && new short[] {3,4,5,9,10}.Contains(p.Type) ?
          ", Value=" + string.Join(",", 
            PropertyItemReader.ReadValues(p).Cast<object>()) : "";

      Array.ForEach(bmp.PropertyItems, p =>
        Console.WriteLine("ID=0x{0:X}({1}), Len={2}, Type={3}({4}){5}",
          p.Id,
          PropertyItemReader.GetExifTagId(p.Id),
          p.Len,
          p.Type,
          PropertyItemReader.GetExifType(p.Type),
          getvalues(p)));
    }
  }
  catch (Exception e)
  {
    Console.WriteLine(e.Message);
  }
}

結局のところ、2つクラス(SwitchActionクラスとSwitchFuncクラス)を作ったけど、使わなかった。まぁ、どっかで使うこともあるかもしれないし、無いかもしれないし。

そして最終的なコードはちょっと微妙なにおいが残ったなぁ。それでもいろいろ書いてみた中では一番ましのような気がするけれど。うーん…。

という、ちょっと歯切れの悪い終わり方でした。あ~あ。

0 件のコメント:

コメントを投稿