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

これの続きー。もう何がなんだか！

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

今日はここから

checkInputCoinsメソッド

TODO: processCoins2でreturnCoinsを計算する。

前回こう書いたけどその前に、おい、俺！checkInputCoinsをよく見なさい。

  void checkInputCoins(List<Integer> aInputCoins) {
    checkNotNull(CollectionUtils.isNotEmpty(aInputCoins),
        "InputCoins must not be null nor empty.");
    checkArgument(aInputCoins.size() <= 100,
        "InputCoins size must be up to 100.");
  }

・・・ごめんなさい。checkNotNullっておかしいですね。checkArgumentですね。

  void checkInputCoins(List<Integer> aInputCoins) {
    checkArgument(CollectionUtils.isNotEmpty(aInputCoins),
        "InputCoins must not be null nor empty.");
    checkArgument(aInputCoins.size() <= 100,
        "InputCoins size must be up to 100.");
  }

checkPriceメソッド

てことで、気を取り直してTODOに進む？・・・んー。その前に、checkPriceメソッドをやっつけちゃおう。

  boolean checkPrice(int sum, Product product) {
    return !(product.getPrice() < sum);
  }

いつも通りに、まずは「!」をなくそう。

  boolean checkPrice(int sum, Product product) {
    return product.getPrice() >= sum;
  }

で、名前をつける。こんな感じか？

  boolean isShortOfMoney(int aSum, Product aProduct) {
    return aProduct.getPrice() >= aSum;
  }

ふむ・・・。おかしい・・・。イコール入ってるのおかしい！こうかっ！

  boolean isShortOfMoney(int aSum, Product aProduct) {
    return aProduct.getPrice() > aSum;
  }

aSumが左の方が意味が伝わりやすいかな。

  boolean isShortOfMoney(int aSum, Product aProduct) {
    return aSum < aProduct.getPrice();
  }

calcSumOfValidCoinsメソッド

てことで、気を取り直してTODOに進む？・・・んー。その前に、前回書いたこれ、

  int calcSumOfValidCoins(List<Integer> aInputCoins) {
    return aInputCoins.stream()
        .filter(coin -> VALID_COINS.contains(coin))
        .mapToInt(i -> i)
        .sum();
  }

calcSumOfValidCoinsって名前にしたけどsumUpValidCoinsのがいっかなと思った。で、そうすると呼び出し元がこんな感じになるんだけど、

  Result b(List<Integer> aInputCoins, Product aProduct) {
    List<Integer> returnCoins = new ArrayList<>();
    int sumOfValidCoins = sumUpValidCoins(aInputCoins);

    if (isShortOfMoney(sumOfValidCoins, aProduct)) {
      return createResult(aInputCoins, null);
    }

    processCoins2(returnCoins, sumOfValidCoins, aProduct);
    return createResult(returnCoins, aProduct);
  }

sumOfValidCoins変数は、やっぱりsumで伝わる気がしてきたのでそうすることにする。

  Result b(List<Integer> aInputCoins, Product aProduct) {
    List<Integer> returnCoins = new ArrayList<>();
    int sum = sumUpValidCoins(aInputCoins);

    if (isShortOfMoney(sum, aProduct)) {
      return createResult(aInputCoins, null);
    }

    processCoins2(returnCoins, sum, aProduct);
    return createResult(returnCoins, aProduct);
  }

うん。悪くない。かな。

TODO: processCoins2でreturnCoinsを計算する。

スッキリしたところで、やっとTODOを始めますかね。まずは、returnCoinsのインスタンス化部分をprocessCoins2の近くに持ってきて良さそうだね。

  Result b(List<Integer> aInputCoins, Product aProduct) {
    int sum = sumUpValidCoins(aInputCoins);

    if (isShortOfMoney(sum, aProduct)) {
      return createResult(aInputCoins, null);
    }

    List<Integer> returnCoins = new ArrayList<>();
    processCoins2(returnCoins, sum, aProduct);
    return createResult(returnCoins, aProduct);
  }

processCoins2の中はこんな感じだから、

  void processCoins2(List<Integer> coins2, int sum, Product product) {
    sum -= product.getPrice();
 
    List<Integer> coins3 = new ArrayList<>();
    coins3.add(500);
    coins3.add(100);
    coins3.add(50);
    coins3.add(10);
    for (Integer coin : coins3) {
      while (sum > coin) {
        coins2.add(coin);
        sum -= coin;
      }
    }
  }

結局これって、processCoins2でreturnCoinsを作って返してあげれば良さそう。よし、引数のcoins2をやめて、中でインスタンス化しよう。

  List<Integer> processCoins2(int sum, Product product) {
    List<Integer> coins2 = new ArrayList<>();
    sum -= product.getPrice();

    List<Integer> coins3 = new ArrayList<>();
    coins3.add(500);
    coins3.add(100);
    coins3.add(50);
    coins3.add(10);
    for (Integer coin : coins3) {
      while (sum > coin) {
        coins2.add(coin);
        sum -= coin;
      }
    }

    return coins2;
  }

もうちょっと後で名前が変わりそうな気もするけど、一旦、適当に名前を考えてみる。

  List<Integer> calcReturnCoins(int aSum, Product aProduct) {
    List<Integer> returnCoins = new ArrayList<>();
    aSum -= aProduct.getPrice();

    List<Integer> coins3 = new ArrayList<>();
    coins3.add(500);
    coins3.add(100);
    coins3.add(50);
    coins3.add(10);
    for (Integer coin : coins3) {
      while (aSum > coin) {
        returnCoins.add(coin);
        aSum -= coin;
      }
    }

    return returnCoins;
  }

引数のaSumを計算に使ってるの嫌だな。こうかな。

  List<Integer> calcReturnCoins(int aSum, Product aProduct) {
    List<Integer> returnCoins = new ArrayList<>();
    int change = aSum - aProduct.getPrice();

    List<Integer> coins3 = new ArrayList<>();
    coins3.add(500);
    coins3.add(100);
    coins3.add(50);
    coins3.add(10);
    for (Integer coin : coins3) {
      while (change > coin) {
        returnCoins.add(coin);
        change -= coin;
      }
    }

    return returnCoins;
  }

で、無効なコインのことを考えると、InputCoinsが必要だから引数に加えて、こうかな。

  List<Integer> calcReturnCoins(List<Integer> aInputCoins, int aSum, Product aProduct) {
    List<Integer> returnCoins = aInputCoins.stream()
        .filter(coin -> not(VALID_COINS.contains(coin)))
        .collect(Collectors.toList());

    int change = aSum - aProduct.getPrice();

    List<Integer> coins3 = new ArrayList<>();
    coins3.add(500);
    coins3.add(100);
    coins3.add(50);
    coins3.add(10);
    for (Integer coin : coins3) {
      while (change > coin) {
        returnCoins.add(coin);
        change -= coin;
      }
    }

    return returnCoins;
  }

coins3の部分をちょっと改善。

  // お釣り計算のため、大きい順に並んでいる必要がある
  static final ImmutableList<Integer> COINS_FOR_CHANGE = ImmutableList.of(500, 100, 50, 10);

  List<Integer> calcReturnCoins(List<Integer> aInputCoins, int aSum, Product aProduct) {
    List<Integer> returnCoins = aInputCoins.stream()
        .filter(coin -> not(VALID_COINS.contains(coin)))
        .collect(Collectors.toList());

    int change = aSum - aProduct.getPrice();

    for (Integer coin : COINS_FOR_CHANGE) {
      while (change > coin) {
        returnCoins.add(coin);
        change -= coin;
      }
    }

    return returnCoins;
  }

うーん。これ、無効なコインの計算と、お釣りの計算で2つやってるから、分けてみようかな。

  List<Integer> getInvalidCoins(List<Integer> aInputCoins) {
    return aInputCoins.stream()
        .filter(coin -> not(VALID_COINS.contains(coin)))
        .collect(Collectors.toList());
  }

  // お釣り計算のため、大きい順に並んでいる必要がある
  static final ImmutableList<Integer> COINS_FOR_CHANGE = ImmutableList.of(500, 100, 50, 10);

  List<Integer> getChangeCoins(int aChange) {
    List<Integer> changeCoins = new ArrayList<>();

    int rest = aChange;
    for (Integer coin : COINS_FOR_CHANGE) {
      while (rest > coin) {
        changeCoins.add(coin);
        rest -= coin;
      }
    }

    return changeCoins;
  }

使う方はこうなった。

  Result b(List<Integer> aInputCoins, Product aProduct) {
    int sum = sumUpValidCoins(aInputCoins);

    if (isShortOfMoney(sum, aProduct)) {
      return createResult(aInputCoins, null);
    }

    int change = sum - aProduct.getPrice();
    List<Integer> invalidCoins = getInvalidCoins(aInputCoins);
    List<Integer> changeCoins = getChangeCoins(change);

    List<Integer> returnCoins = ListUtils.union(invalidCoins, changeCoins);
    return createResult(returnCoins, aProduct);
  }

こんなもんかな。

・・・で、全体をざっとみて。bメソッドはbuyProductって名前にしようかな。
aメソッドは。。。要らん気がしてきた。executeに処理を戻そうかな。

それと、最後に、createResultをresultOfにして2つに分けてみた。

  Result resultOf(Product aProduct, List<Integer> aReturnCoins) {
    Result result = new Result();
    result.setProduct(aProduct);
    result.setCoins(aReturnCoins);
    return result;
  }

  Result resultOf(List<Integer> aInputCoins) {
    Result result = new Result();
    result.setProduct(null);
    result.setCoins(aInputCoins);
    return result;
  }

こうなった。今日はここまでー。

一晩寝かせて、明日の朝起きて、どう思うかなー。
明日はまとめができるといいなー。

っても、まだバグあるよね。さて・・・。