Java8でJava8っぽいコードを書く

概要(お前は何を言っているんだ)

• コードレビューしていると、__Java8のfeature(Stream APIとかOptionalとか)を使ってるんだけど、Java7以前と大差ない書き方をされているコード__をよく見かける。
• もっと皆に__Java8っぽいコード__を書いて欲しい。そうじゃないともったいない。
  (*"Java8っぽい"の基準は割りと(かなり?)私見が入っている)
• 言葉を尽くしても伝わりづらいので、例を作ろう。 ← これ
  • ついでに、「無理してJava8のfeature使うことで、逆に悪くなってしまっている例」も作ろう。

対象読者

• Java8でコード書いてるけど、Java7以前の書き方が染み付いちゃっていてなかなか抜け出せない人
• 「forやnullチェックは絶対使うべきでない」という思いが強すぎて、何でもかんでもStream APIやOptional使ってる人

Notice

この記事で書いてある内容は特に目新しいものではないし、Java以外の言語だともっと洗練された書き方もできるものもある。
この記事はあくまで「Java7以前からJava8に移行したけどStreamやOptionalを使った書き方に慣れない人」のための記事である。

もっとJava8っぽくできるパターン

こんな感じの商品オブジェクトがあって、それに対して何らかの処理を行うという想定。

Item.java

// 商品クラス
public class Item {
  String name; // 商品名
  BigDecimal price; // 商品の価格
  String currency; // priceフィールドの通貨("JPY"とか"USD")
}

拡張for文をforEachで置き換えただけ

「あるデータを変換して別リストに格納する」みたいな処理をしたいとする。
Java7以前だと↓のようになる。

java7以前の書き方

// 日本円の商品をUSDに変換
BigDecimal jpyToUsd = BigDecimal.valueOf(100);

List<Item> usdItems = new ArrayList<>();

for(Item jpyItem : jpyItems) {
    Item usdItem = new Item();
    usdItem.setName(jpyItem.getName());
    usdItem.setPrice(jpyItem.getPrice().multiply(jpyToUsd));
    usdItem.setCurrency("USD");
    usdItems.add(usdItem);
}

で、「Java8なんだからfor使わずにStreamでやれ」って言われた結果、こうなる。

forEachだけ

BigDecimal jpyToUsd = BigDecimal.valueOf(100);

List<Item> usdItems = new ArrayList<>();

jpyItems.forEach(jpyItem -> {
    Item usdItem = new Item();
    usdItem.setName(jpyItem.getName());
    usdItem.setPrice(jpyItem.getPrice().multiply(jpyToUsd));
    usdItem.setCurrency("USD");
    usdItems.add(usdItem);
});

やってることが拡張for文と全く変わっていない。
これを、Streamの"中間操作"と"終端操作"を意識して書き変えると、こうなる。

Java8っぽい

BigDecimal jpyToUsd = BigDecimal.valueOf(100);

// forEachを使わず、mapとcollectの組み合わせで実現
List<Item> usdItems = jpyItems.stream()
        .map(jpyItem -> {
            Item usdItem = new Item();
            usdItem.setName(jpyItem.getName());
            usdItem.setPrice(jpyItem.getPrice().multiply(jpyToUsd));
            usdItem.setCurrency("USD");

            return usdItem;
        })
        .collect(Collectors.toList());

(* mapの中身がfatになっていることについての対応策については後述する)

こうすることにより、

• あるデータを変換して別のデータを生成している(map(...))
• 変換されたデータをまとめてCollectionにしている(collect(...))

ということがより明確になるので、可読性が上がる。
forEachは何でもできて便利だが、他に適切なメソッドがないかどうか考えてみよう。

lambdaがデカい

先程の例だとmapの中のlambdaが大きくて、Streamのメソッドチェーンが長くなってしまい全体が見えづらく可読性が低い。

「じゃあlambdaを変数として取り出せばいいのか」とすると、こうなる。

lambdaを変数化

Function<Item,Item> convertToUsdItem = jpyItem -> {
    Item usdItem = new Item();
    usdItem.setName(jpyItem.getName());
    usdItem.setPrice(jpyItem.getPrice().multiply(jpyToUsd));
    usdItem.setCurrency("USD");

    return usdItem;
};

List<Item> usdItems = jpyItems.stream()
    .map(convertToUsdItem)
    .collect(Collectors.toList());

これでもStreamはスッキリするんだけど、これだとconvertToUsdItemの処理をテストしたいとかなったときのテスタビリティ低いので、通常メソッドにしたほうが何かと便利だし、個人的には読みやすい。
(可読性については好みの問題かもしれない)

通常メソッドとして切り出す

public Item convertToUsdItem(Item jpyItem) {
    Item usdItem = new Item();
    usdItem.setName(jpyItem.getName());
    usdItem.setPrice(jpyItem.getPrice().multiply(jpyToUsd));
    usdItem.setCurrency("USD");

    return usdItem;
}

List<Item> usdItems = jpyItems.stream()
    .map(this::convertToUsdItem)
    .collect(Collectors.toList());

メソッド参照を利用すれば、lambdaと同じくらいStreamをシンプルに記述できる。

対象オブジェクトの一部フィールドだけ使ってるのにmapしていない

例えば、「商品名が"A"で始まる商品について、商品名を標準出力する」みたいな処理を考えたとき、こんな感じのコードをたまに見かける。

items.steam()
    .filter(item -> item.getName().startsWith("A"))
    .forEach(item -> System.out.println(item.getName()));

商品名だけ使いたいのにitemオブジェクトを最後まで引き回す必要はないので、最初にmapでnameフィールドを取り出しておいたほうが後続処理がシンプルになる。

mapでnameだけ取り出して処理

items.stream()
    .map(Item::getName)
    .filter(name -> name.startsWith("A"))
    .forEach(System.out::println);

こうすることで、

• ”商品名のみを扱う”ということが早い段階で理解できる
• メソッド参照によりコードをよりシンプルにできる

といったメリットがある。

nullを返すメソッドを作って、その呼び元でnullチェックしている

Stream APIのfindFirstの戻り値に対する処理や、Java8用ライブラリのメソッドでOptionalが返ってくるものを使うときなどは、否が応でもOptionalを使わないといけない。
が、Java8を学びたての頃は、自分でメソッドを実装しようとするときに「ここは戻り値の型をOptionalにしよう」とは思い至らないことが多いように見受けられる。

例えば「キャッシュから為替レート情報を取得して、もしキャッシュが存在しなければデフォルト値(1)を返す」みたいなメソッドを作ったとする。

キャッシュから為替レートを取得する(null版)

public RateInfo getFromCache(String fromCurrency, String toCurrency) {
    // キャッシュに存在すればそのRateInfoオブジェクトを、なければnullを返す
}

public BigDecimal retrieveRate(String fromCurrency, String toCurrency) {

    RateInfo rateInfo = getFromCache(fromCurrency, toCurrency);

    if(rateInfo != null) {
        return rateInfo.getRate();
    }

    return BigDecimal.ONE;
}

ここでgetFromCacheメソッドは、「キャッシュに存在すればそのRateInfoオブジェクトを、なければnullを返す」という挙動をする。
なので、呼び元でnullチェックのためのif文が必要になる。
で、たまにそのnullチェックをしてなかったりしてぬるぽったりする。

というわけで、ここでOptionalの出番。

キャッシュから為替レートを取得する(Optional版)

public Optional<RateInfo> getFromCache(String fromCurrency, String toCurrency) {
    // キャッシュからRateInfoオブジェクトを取り出そうとした結果を、Optional<RateInfo>オブジェクトとして返す
}

public BigDecimal retrieveRate(String fromCurrency, String toCurrency) {
    Optional<RateInfo> rateInfo = getFromCache(fromCurrency, toCurrency);

    return rateInfo.map(RateInfo::getRate).orElse(BigDecimal.ONE);
}

こうすることで、

• nullチェックというかオブジェクトの存在チェックをメソッドの呼び元に強制できる
  • rateInfo.get()でチェックをせずに無理やり値を取り出すこともできるが、そういったコードは静的解析で弾ける
• orElseなどのメソッドを使用することでコードをすっきり書ける

などのメリットがある。

無理してJava8のfeatureを使わないほうが良いパターン

__forとnullチェック絶対殺すマン__みたいになってると陥りやすいケース。
Java8のStreamとかOptionalは他言語のものに比べてイケてないことが多くて、そこを無理矢理なんとかカバーしようとするとドツボにハマる。

indexつきfor文をStreamで置き換える

たとえば、

indexつきfor文

for(int i=0; i < items.size(); i++) {
    System.out.println(
        String.valueOf(i) + " : " + items.get(i).getName());
}

のような処理について、「for文なくしてやるぞー！」って気合い入れた結果、

Stream使って頑張る

// IntStreamでループカウンタ生成
IntStream.range(0, items.size())
    .forEach(i -> System.out.println(
        String.valueOf(i) + " : " + items.get(i).getName());

// AtomicIntegerとforEachで頑張る
AtomicInteger i = new AtomicInteger();
items.forEach(item -> System.out.println(
        String.valueOf(i.getAndIncrement()) + " : " + item.getName());

みたいになってしまう。
前者は、本来はitemsのStream処理として表現したいのに、index生成用のIntStreamの中でget(i)使ってアクセスしているので、通常のfor文と変わらないし、読みづらい。
(筆者は以前この書き方をしていたが、上述の理由から止めた)

後者は後者で、「プリミティブ型のintはlambda内でインクリメントできないから、AtomicInteger使う」ってところが、AtomicIntegerの本来的な使い方から外れてる気がするのでオススメしない。

JavaにindexつきのCollection操作（Kotlinのこれなど）があればそれを使えばいいんだけど、
あいにくまだ実装されていないので、無理やりStream APIつかうよりは素直にfor文使っていたほうが今のところは良さそう。

nullチェックのためだけのOptional.ofNullable

古いライブラリやJava7以前でも利用可能なメソッドの中には、戻り値としてnullを返し得るものもある。
そういったメソッドについて、「その戻り値nullかどうかだけを確認したいだけ」な場合に、「何が何でもif (a != null)は書かねーぞ！」という気持ちになると下記のようなコードが出来上がる。

戻り値をOptionalで包んで評価する

// getText()はnullを返し得る
String text = getText();

// Optionalで包んでチェック。中身の値は使わない
if(Optional.ofNullable(text).isPresent()) {
    return "OK";   
}

return "NG";

Optional.isPresent()使っているというところもよろしくないが、普通にnullチェックするよりもコードが長くなって若干煩くなってしまっている。
ここは素直にif(text != null)で良いと思う。

ちょっと悩み中な部分 - デフォルト引数のような使い方

Optionalは設計思想として「メソッドの戻り値」として使用することを主眼としていて、それ以外の用途（たとえば引数の型として用いる）などは推奨されていないそう。
r.f stackoverflow - Why java.util.Optional is not Serializable, how to serialize the object with such fields

確かに引数にOptional型を使っても、その渡ってきたOptionalオブジェクトがnullな場合もあるので、あまり旨味がない。
とは言え、引数に関して「こういうOptionalの使い方できるんじゃね？」というものがある。
それが__デフォルト引数__のような使い方だ。

Optionalで擬似的なデフォルト引数を表現する

// 対象商品を指定した通貨の価格に変換したものを返す
public Item convert(Item item, String toCurrency) {
     // 通貨の指定がなければUSDを使用する
     String currency = Optional.ofNullable(toCurrency).orElse("USD");

     ...
     ...
}

「overload使えよ」的なご意見もあるだろうが、サードパーティライブラリのInterfaceの実装メソッドでコールバックメソッドとして指定するものなどにはoverload使えないので、こういう使い方もアリなんじゃないかなと検討中。

追記(2017/08/30)

コメントでご指摘いただいたとおり、別のメソッドで対応したほうがいいかも。

「nullであればデフォルト値」系メソッドを使う

// 対象商品を指定した通貨の価格に変換したものを返す
public Item convert(Item item, String toCurrency) {
     // java.util.Objects.toString
     String currency1 = Objects.toString(toCurrency,"USD");

     // org.apache.commons.lang3.ObjectUtils.defaultIfNull
     String currency2 = ObjectUtils.defaultIfNull(toCurrency, "USD");

     ...
     ...
}

最後に

• Stream APIもOptionalも、「単なるメソッド・クラスの追加」ではなく「今までよりも効率的で理解しやすい書き方ができる」と捉えて、色々な書き方を探ってみると面白い。
• とは言え、Java8時点ではイケてない部分も多いので、盲目的に使用するのではなく「丁度良い使い方」を探っていくのが肝要。