java - Using Optional to prevent NullPointers In TrainWrecks

Question

I was just wondering if the following is the wrong use case for Optional. It probably is, because it looks nasty.

(It is regarding calling "Legacy" code that returns nulls.)

  private static class Person {
    private Address address;
    private String name;
  }

  private static class Address {
    private Integer housenr;
    private String houseletter;
    private String street;
  }

  public String getAddress(Person person) {
    return Optional.ofNullable(person.getAddress())
      .map(x -> Optional.ofNullable(x.getHousenr()).map(y -> y + " ").orElse("") +
          Optional.ofNullable(x.getHouseletter()).map(y -> y + " ").orElse("") +
          Optional.ofNullable(x.getStreet()).orElse(""))
      .orElse("<unknown>");
  }

  // unit test if string representation of an address is properly generated
  assertThat(getAddress(charlesBabbage))
      .isEqualTo("4 l Regentstreet");

I should probably put the code in a method of the Person class and the Address class, so it doesn't bother me so much.

Or should I do it "the old way":

  public String getAddress(Person person) {
    if (person.getAddress() == null) {
      return "<unknown>";
    }
    StringBuilder builder = new StringBuilder();
    if (person.getAddress().getHousenr() != null) {
      builder.append(person.getAddress().getHousenr() + " ");
    }
    if (person.getAddress().getHouseletter() != null) {
      builder.append(person.getAddress().getHouseletter() + " ");
    }
    if (person.getAddress().getStreet() != null) {
      builder.append(person.getAddress().getStreet() + " ");
    }
    return builder.toString();
  }

Bear in mind that this is just an example. More fields can be added like affix., postoffice box, town/city, municipality, state, country (not to mention foreign addresses) exacerbating the problem.

Answer 1

In both your examples, you are repeating code that checks that elements are not null before adding them to a String. You can reduce this repetitive work by using Stream and joining:

public String getAddress(Person person) {
    return Optional.ofNullable(person.getAddress())
            .map(x -> Stream.of(x.getHousenr(), x.getHouseletter(), x.getStreet())
                        .filter(Objects::nonNull)
                        .map(Object::toString)
                        .collect(Collectors.joining(" "))
            )
            .orElse("<unknown>");
}

Or similarly without Optional:

public String getAddress(Person person) {
    Address address = person.getAddress();
    if (address == null) {
        return "<unknown>";
    }
    return Stream.of(address.getHousenr(), address.getHouseletter(), address.getStreet())
            .filter(Objects::nonNull)
            .map(Object::toString)
            .collect(Collectors.joining(" "))
}