Indentera mindre: Ett enkelt tips för att få mer läsbar kod

Squeeds Julkalender | 2019-12-03 | Magnus Larsson
Är du trött på nästlad kod med komplexa uttryck? Snurrar ögonen i sina glober när du ser kod med fyra for-loopar i varandra som alla muterar tillstånd på olika sätt? Leta inte längre, här kommer en guide för att räta ut (Java)-koden!
messy_wires.jpg

Syftet med den här korta guiden kommer vara att använda Optional för att optimera läsbarhet, främst genom att minska mängden indentering men också genom att bryta ut nästlad kod i hjälpmetoder.

Vi börjar med ett svåröverskådligt exempel och förbättrar det stegvis. Det röriga exemplet nedan mappar ett internt modellobjekt till ett api-objekt:

public ApiDevice mapDbObjectToApiObject(Device device) {
  ApiDevice.ApiDeviceBuilder builder = ApiDevice.builder();
  builder.name(device.getName());

  if (device instanceof BatteryPoweredDevice) {
      BatteryPoweredDevice batteryPoweredDevice = (BatteryPoweredDevice)device;
      builder.batteryLifeTime(batteryPoweredDevice.getLifeTime());

      if (batteryPoweredDevice.getStatus() != null && batteryPoweredDevice.getStatus().hasValue()) {
          builder.batteryStatus(batteryPoweredDevice.getStatus().getValue());
      }
  }
  return builder.build();
}

Optional till undsättning!

Svårt att läsa, eller hur? Av de saker som främst sticker ut finns två nästlade if-satser. Den första if-satsen evaluerar om ett objekt kan konverteras till en BatteryPoweredDevice. I ett sådant läge passar det bra att linda in objektet i en Optional och sedan filtrera ut det baserat på instanstyp. Precis som Stream så innehåller Optional både filter och map, vilka kan användas i detta läget.

Vi kan bryta ut det till följande metod:

private Optional<BatteryPoweredDevice> getBatteryPoweredDevice(Device device) {
  return Optional.of(device) // Use ofNullable if device can be null
          .filter(d -> d instanceof BatteryPoweredDevice)
          .map(d -> (BatteryPoweredDevice) d);
}

En av de bästa egenskaperna hos en Optional är att map evaluerar huruvida det finns ett värde innan den applicerar en funktion. Det betyder att man inte behöver oroa sig för NullPointerException om man mappar en variabelnivå i taget! Vi använder det faktumet för att få fram batteryStatusValue från exemplet ovan.

private Integer getBatteryStatusValue(Device device) {
  return getBatteryPoweredDevice(device)
          .map(BatteryPoweredDevice::getStatus) // Nivå 1
          .filter(BatteryStatus::hasValue)
          .map(BatteryStatus::getValue) // Nivå 2
          .orElse(null);
}

För att extrahera ut värdet från en Optional bör man använda orElse (för defaultvärden) eller orElseThrow (om det inte finns ett defaultvärde). Att använda get(), vilket på pappret kan se enklast ut, är bäst att endast göra i situationer där man absolut vill fallera om Optional inte innehåller ett värde, till exempel i test. Nedan använder vi orElse för att returnera ett tomt värde.

private Long getBatteryLifeTime(Device device) {
  return getBatteryPoweredDevice(device)
          .map(BatteryPoweredDevice::getLifeTime)
          .orElse(null);
}

Slutresultat

Vi använder våra nya metoder för att rensa upp den gamla koden. Slutresultatet indenteras max ett steg, vilket gör att det blir betydligt lättare att läsa.

private ApiDevice toApiDevice(Device device) {
  return ApiDevice.builder()
          .name(device.getName())
          .batteryStatus(getBatteryStatusValue(device))
          .batteryLifeTime(getBatteryLifeTime(device))
          .build();
}

private Integer getBatteryStatusValue(Device device) {
  return getBatteryPoweredDevice(device)
          .map(BatteryPoweredDevice::getStatus)
          .filter(BatteryStatus::hasValue)
          .map(BatteryStatus::getValue)
          .orElse(null);
}

private Long getBatteryLifeTime(Device device) {
  return getBatteryPoweredDevice(device)
          .map(BatteryPoweredDevice::getLifeTime)
          .orElse(null);
}

private Optional<BatteryPoweredDevice> getBatteryPoweredDevice(Device device) {
  return Optional.of(device) // Use ofNullable if device can be null
          .filter(d -> device instanceof BatteryPoweredDevice)
          .map(d -> (BatteryPoweredDevice) d);
}

Att tänka på

Optional passar extra bra då man filtrerar bort vissa resultat och behåller andra, vilket i klassisk java kan likställas vid en ren if-sats. Om man däremot vill ha en if-else så borde man se över om man faktiskt inte hellre vill behålla det som en if-else.

Även om man kommer långt med map och filter så tillhandahåller Optional många fler funktioner, så det är värt att ta ytterligare en kik på vad man mer kan göra. Vill man ha en snabb guide kring metoderna i Optional och hur man använder (och inte använder) dem så finns det en bra guide här. Vill man ha ännu mer inspiration kan man ta en kik på dokumentationen för Stream, Mono och Flux.