読者です 読者をやめる 読者になる 読者になる

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

これの続きです!

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

注意点

ゴールが決まってるわけじゃなくて、試行錯誤しながら進むので、その途中で「これが正解か!」って思わないようにね。
最終的に「やっぱあんときのあのメソッド切り出しはナシだわ!」ってなる可能性も高いです。

※ 初心者の方向けのコメントです。途中経過だけ読んで「なるほど!」って鵜呑みにすると申し訳ないので。
※ 自分の中に答えがある人は、適当に(・∀・)ニヤニヤしたりコメントくれたりするとうれしす。

前回までのあらすじ

TransactionScriptな手続き型のプログラムでも、メソッドを切ることで意図が伝わってくるようにできたりするよね?
僕はそういう、読んで意図が伝わってくるコードが好きです。

ということで、どういう風に考えながらコードを書いてるかなーってのを書いてみようと思って。
自分ルールとしては「同時に2つのことをしない。1歩ずつやる。」という感じです。

1. 最初に、構造を読みやすくしました。この時点ではメソッド切り出しを考えない。
2. 次に、メソッド切り出しをしました。この時点ではメソッド名は考えない。
3. で、全体の流れが見えてきたので、今回は、処理の意味を考えてみようと思います。

今日はここから


メイン処理

メインの処理全体がtry-catchされてるところがちょっと読みにくいのでメソッドに切り出してみようかな。
メソッド切り出しはIDEの機能でサクッとね。

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

  Result a(Input in) {
    if (isInvalidSelected(in.getSelected())) {
      throw new IllegalArgumentException();
    }

    if (isInvalidCoins(in.getCoins())) {
      throw new IllegalArgumentException();
    }

    List<Integer> coins2 = new ArrayList<>();
    int sum = processCoins(in.getCoins(), coins2);

    Product product = dao.findById(in.getSelected());
    if (product == null) {
      return createResult(in.getCoins(), null);
    }

    if (checkPrice(sum, product)) {
      return createResult(in.getCoins(), null);
    }

    processCoins2(coins2, sum, product);
    return createResult(coins2, product);
  }

うん。僕はこの方が好きだな。思考の流れが
1. 処理全体をtry-catchで囲んでるんだな。ふむふむ。
2. で、処理はどうなってるのかな?
って進むことができるので。

入力値チェックをまとめたい

こうして見てみると、入力値チェックが半分を占めてるのだけど、これってビジネスロジックに直接関係ないから分けて考えたい。
そう考えると、以下の部分の処理が逆にできると幸せっぽい。前半はビジネスロジック、後半は入力値チェックみたいだからね。

    List<Integer> coins2 = new ArrayList<>();
    int sum = processCoins(in.getCoins(), coins2);

    Product product = dao.findById(in.getSelected());
    if (product == null) {
      return createResult(in.getCoins(), null);
    }

入れ替えると何が変わるかな?
in.getCoins()がnullのときにNPEが発生しそうだけど、これはその前の入力値チェックでチェックしてるから大丈夫そう。
入れ替えるか。

  Result a(Input in) {
    if (isInvalidSelected(in.getSelected())) {
      throw new IllegalArgumentException();
    }

    if (isInvalidCoins(in.getCoins())) {
      throw new IllegalArgumentException();
    }

    Product product = dao.findById(in.getSelected());
    if (product == null) {
      return createResult(in.getCoins(), null);
    }

    List<Integer> coins2 = new ArrayList<>();
    int sum = processCoins(in.getCoins(), coins2);

    if (checkPrice(sum, product)) {
      return createResult(in.getCoins(), null);
    }

    processCoins2(coins2, sum, product);
    return createResult(coins2, product);
  }

ロジックを切り出してみる

これで前半が入力値チェックに、後半がビジネスロジックになったかな。
後半をメソッドに切り出してみる。

  Result a(Input in) {
    if (isInvalidSelected(in.getSelected())) {
      throw new IllegalArgumentException();
    }

    if (isInvalidCoins(in.getCoins())) {
      throw new IllegalArgumentException();
    }

    Product product = dao.findById(in.getSelected());
    if (product == null) {
      return createResult(in.getCoins(), null);
    }

    return b(in.getCoins(), product);
  }

  Result b(List<Integer> inputCoins, Product product) {
    List<Integer> coins2 = new ArrayList<>();
    int sum = processCoins(inputCoins, coins2);

    if (checkPrice(sum, product)) {
      return createResult(inputCoins, null);
    }

    processCoins2(coins2, sum, product);
    return createResult(coins2, product);
  }

・・・なんとなく、やり過ぎな感じがしてきた。前半の入力チェックの部分をメインのメソッドに戻してみるか?
いや、まいっか。

ということで、今はこうなってます。終わらん!(ノД`)シクシク

次はメソッドの中を綺麗にしてく楽しい作業なはずー。今週は一週間、これやって終わらせるぞー!

追記

続き書いた。bufferings.hatenablog.com