続々々々：僕の好きなコードの書き方

これの続きー。ぞくぞくしてきたぞー。

僕の好きなコードの書き方 - Mitsuyuki.Shiiba
続：僕の好きなコードの書き方 - Mitsuyuki.Shiiba
続々：僕の好きなコードの書き方 - Mitsuyuki.Shiiba
続々々：僕の好きなコードの書き方 - Mitsuyuki.Shiiba

今日はここから

最初のメソッドについて考える

  public Result execute(Input in) {
    try {
      return a(in);
    } catch (Exception e) {
      LOG.error(e);
      return createResult(in.getCoins(), null);
    }
  }

ふむ。これinがnullのときはaメソッドの中でNPEが発生してcatchの中でさらにNPEが発生するね。これでいいんかな？どうしよう。
そもそもこのexecuteメソッドのパラメータがどういう状態で渡されてくるんだろうか・・・。とりあえず、早めにNPEにしとくか。

  public Result execute(Input in) {
    if (in == null) {
      throw new NullPointerException("In is null.");
    }

    try {
      return a(in);
    } catch (Exception e) {
      LOG.error(e);
      return createResult(in.getCoins(), null);
    }
  }

ふむ。これでちょっとやりやすくなった。

aメソッドのパラメータを変えてみる

なんとなく、aメソッドをinから解放したかったので。こうしてみた。

  public Result execute(Input in) {
    if (in == null) {
      throw new NullPointerException("In is null.");
    }

    try {
      return a(in.getSelected(), in.getCoins());
    } catch (Exception e) {
      LOG.error(e);
      return createResult(in.getCoins(), null);
    }
  }

  Result a(Integer aSelected, List<Integer> aCoins) {
    if (isInvalidSelected(aSelected)) {
      throw new IllegalArgumentException();
    }

    if (isInvalidCoins(aCoins)) {
      throw new IllegalArgumentException();
    }

    Product product = dao.findById(aSelected);
    if (product == null) {
      return createResult(aCoins, null);
    }

    return b(aCoins, product);
  }

aメソッドの中で考えることが、少しだけ減って幸せ。

名前を考える

さて、ついに名前を考え始めようか。
aSelectedとaCoinsだけど、もうちょい分かりやすく。こうかな？

  Result a(Integer aProductId, List<Integer> aInputCoins) {
    if (isInvalidSelected(aProductId)) {
      throw new IllegalArgumentException();
    }

    if (isInvalidCoins(aInputCoins)) {
      throw new IllegalArgumentException();
    }

    Product product = dao.findById(aProductId);
    if (product == null) {
      return createResult(aInputCoins, null);
    }

    return b(aInputCoins, product);
  }

isInvalidSelected

次はisInvalidSelected。

  boolean isInvalidSelected(Integer selected) {
    return !(selected != null && selected > 0 && selected < 11);
  }

こうかな？

  boolean isNotValidProductId(Integer aProductId) {
    return !isValidProductId(aProductId);
  }

  boolean isValidProductId(Integer aProductId) {
    if(aProductId == null) {
      return false;
    }

    int productId = aProductId.intValue();
    return (0 < productId && productId < 11);
  }

冗長な気もするけど。「〜でない」ときを考えるより「〜である」ときを考える方が好きなので。

isInvalidCoins

同じ感じで。

  boolean isInvalidCoins(List<Integer> coins) {
    return !(coins != null && coins.size() > 0 && coins.size() < 101);
  }

こうしてみた。

  boolean isNotValidInputCoins(List<Integer> aInputCoins) {
    return !isValidInputCoins(aInputCoins);
  }

  boolean isValidInputCoins(List<Integer> aInputCoins) {
    if(aInputCoins == null) {
      return false;
    }

    int coinCount = aInputCoins.size();
    return (coinCount > 0 && coinCount < 101);
  }

Productを取得する部分

この部分を見て。ちょっと悩む。

    if (isNotValidProductId(aProductId)) {
      throw new IllegalArgumentException();
    }

    if (isNotValidInputCoins(aInputCoins)) {
      throw new IllegalArgumentException();
    }

    Product product = dao.findById(aProductId);
    if (product == null) {
      return createResult(aInputCoins, null);
    }

Productが見つからなかった時は、createResultしてるんだけど。
パラメータが不正な場合は例外投げてて。最終的にはどっちも同じ結果を返すのよね。。。

1) 入力値エラーの場合も、createResultする。
2) Productが見つからない場合も、例外投げる。
3) そのまま

んー。どうしよう？(2)にしようかな。gotoっぽい？いいや。

  Result a(Integer aProductId, List<Integer> aInputCoins) {
    if (isNotValidProductId(aProductId)) {
      throw new IllegalArgumentException("ProductId is not valid.");
    }

    if (isNotValidInputCoins(aInputCoins)) {
      throw new IllegalArgumentException("InputCoins is not valid.");
    }

    Product product = dao.findById(aProductId);
    if (product == null) {
      throw new IllegalArgumentException("Product doesn't exist.");
    }

    return b(aInputCoins, product);
  }

bメソッドに進む前に

ちょこっと整理したい。
やっぱり、isNotValidメソッドが冗長な感じがするので、こうしてみる？

  Result a(Integer aProductId, List<Integer> aInputCoins) {
    if (!isValidProductId(aProductId)) {
      throw new IllegalArgumentException("ProductId is not valid.");
    }

    if (!isValidInputCoins(aInputCoins)) {
      throw new IllegalArgumentException("InputCoins is not valid.");
    }

    Product product = dao.findById(aProductId);
    if (product == null) {
      throw new IllegalArgumentException("Product doesn't exist.");
    }

    return b(aInputCoins, product);
  }

でも僕「!」好きくないので。こういうユーティリティクラスを用意することにする。

public class Not {
  public static boolean not(boolean aBoolean) {
    return !aBoolean;
  }
}

で、staticインポートして使う。

  Result a(Integer aProductId, List<Integer> aInputCoins) {
    if (not(isValidProductId(aProductId))) {
      throw new IllegalArgumentException("ProductId is not valid.");
    }

    if (not(isValidInputCoins(aInputCoins))) {
      throw new IllegalArgumentException("InputCoins is not valid.");
    }

    Product product = dao.findById(aProductId);
    if (product == null) {
      throw new IllegalArgumentException("Product doesn't exist.");
    }

    return b(aInputCoins, product);
  }

で、guava使おうかなとか、Coinクラスを作りたいな、とか思いながら。今日はおしまい。

イマココ

追記

続きbufferings.hatenablog.com