wissel.net

Usability - Productivity - Business - The web - Singapore & Twins

Java style guide: Functions, Streams and Optionals


Over the years I developed a certain style developing in Java (and other languages) that made my code more readable. This isn't about the formatting style guide (Just stick to Google) or making code spotless, but about style.

Objectives

Code needs to be testable, easy to adopt and understandable when someone else, like your future self, tries to read it. Starting with SOLID principles, the guidelines help to get there. They work for any style of development, while I strongly recommend looking at TDD, Dave agrees, so does Allen. In no particular order, let's dig in.

Favour Optional returns over null

When you use Optional as a return type, you can/need to be specific in your code what to do with the return value. It also makes it clearer during development that a NPE is a flaw in code and not a failed lookup. Compare the two snippets:

// CustomerService returns null when not found
final Customer customer = CustomerService.lookup(id);
if (customer != null) {
  sendWelcomeMail(customer, mailBody);
}

In Optional style:

// CustomerService returns Optional.empty when not found
CustomerService.lookup(id)
    .ifPresent(customer -> sendWelcomeMail(customer, mailBody));
}

Split infrastructure code from business logic

The part of the code that deals with connectivity, casting of objects, routing of data etc. should be kept apart from actual business logic. This depends on experience:

   int runCampaign(List<Customers> customers) {
      int result = 0;
      String mailBody = CampaingService.highValueCampaign();
      for (Customer customer: customers) {
        if (customer.isPremium() || (customer.revenue() > 10000 && customer.years() > 3)) {
          if sendCampaignMail(customer, mailBody) {
            result++;
          }
        }
      }
      return result;
   }

Split style with Streams (more variations coming)

   int runCampaign(List<Customers> customers, String mailBody) {
    return Customers.stream()
      .filter(this::eligibleCustomer)
      .filter(customer -> sendCampaignMail(customer, mailBody))
      .count();
   }

   boolean eligibleCustomer(Customer customer) {}
        return customer.isPremium()
         || (customer.revenue() > 10000 && customer.years() > 3));
   }

You can further make this code more testable by moving the sendCampaignMail method to a Campaign object, that you mock when testing:

   int runCampaign(List<Customers> customers, Campaign campaign) {
    return Customers.stream()
      .filter(this::eligibleCustomer)
      .filter(campaign::sendCampaign)
      .count();
   }

   interface Campaign() {
      // sending could be any channel, not just mail
      boolean sendCampaign(Customer customer);
   }

   class CampaignManager() { // Technically its a factory
      static Campaign getMailCampaign(String mailBody, MailOptions options);
   }

Having bussiness logic (eligibleCustomer) extracted, allows to test it individually in isolation. The example is a pure function which I tend to keep as a static method. When criteria depend on e.g. campaign parameters, the method would move to the campaign instance.

There's a little "sin" in my use of the stream for which I probably roast in the ninth circle of hell some time. The filter method sendCampaign has a "side effect" which happens to be its main purpose. I'm also guilty of using map methods for side effects. Practicallity over pure teachings:

   List<CampaignResults> runCampaign(List<Customers> customers, Campaign campaign) {
    return Customers.stream()
      .filter(campaign::eligibleCustomer)
      .map(campaign::executeCampaign)
      .collect(Collectors::toList);
   }

   interface Campaign() {
      CampaignResults executeCampaign(Customer customer);
      boolean eligibleCustomer(Customer customer);
   }

Favour streams over classical loops

Classic loops are an example of imperative programming, while streams fall into the declarative programming category, read about the differences. You can add functional programming to the mix, which can use elements of both.

When squeezing out the last bit of speed and performance, imperative wins. Unless you work on a Google scale project, your loop's speed gain most likely is a case of premature optimization. My goal is readability. Instead of loop bodies riddled with if and case statements, I use streams with map and filter statements. I use filters with side effects in place of a case structure (another option: function maps, see below).

final List<CampaignResults> results = new ArrayList<>(); // use syncronized Collection for parallel streams
customers.stream()
    .filter(customer -> campaign.typeACustomers(results, customer))
    .filter(customer -> campaign.typeBCustomers(results, customer))
    .filter(customer -> campaign.typeCCustomers(results, customer))
    .count(); // Provides unprocessed customers

There's more about using map and Collectors. One sticky problem, where loops seem to have the upper hand is the fact that a mapping operation returns one value and the input to a map is "lost". Lets say: you have a Customer instance and need to fetch CampaignOptions based on CustomerStatus and use that as input for the campaign execution. The code would look like this:

   List<CampaignResults> runCampaign(List<Customer> customers, Campaign campaign) {
    return customers.stream()
      .filter(campaign::eligibleCustomer)
      .map(customer -> {
        final CampaignOptions options = campaign.getOptions(customer.status());
        return campaign.executeCampaign(customer, options);
      })
      .collect(Collectors::toList);
   }

Readability suffered. With an introduction of an intermediate class we can gain it back.

public Class Pair<p, s>() {
  final Optional<p> primary;
  final Optional<s> secondary;

  public Pair<p, s>(p o1, s o2) {
    this.primary = Optional.of(o1);
    this.secondary = Optional.of(o2);
  }
}

   List<CampaignResults> runCampaign(List<Customer> customers, Campaign campaign) {
    return customers.stream()
      .filter(campaign::eligibleCustomer)
      .map(campaign::getOptionsByCustomer)
      .map(campaign::executeCampaignWithOptions)
      .collect(Collectors::toList);
   }

   Interface campaign {
    Pair<Customer,CampaignOptions> getOptionsByCustomer(Customer customer);
    CampaignResults executeCampaignWithOptions(Pair<Customer,CampaignOptions> source);
   }

The constructor with Objects isn't very elegant, so you might opt creating specialized classes (a.k.a DTO) (also see update below).

Function Maps

I'm not a big fan of case structures, they invite too easily stuffing code inbetween the case keywords and thus impacting readability badly. With the release of Java 8 in 2014, Java gained functional interfaces making methods first class citizens (kind of). This allows for alternatives to switch statements


void onTrafficLightChange(TrafficLight light) {
  switch (light) {
    case TrafficLight.RED:
       stopVehicle();
       stopEnginIfYourCarIsSmart();
       if (country == "Singapore") {
          proceedOnLeftTurn();
       } else {
          proceedObRightTurn();
       }
       break;
       ...
  }
}

instead define one function for the phase:

Consumer<TrafficLight> onRed = (light) -> {...};
Consumer<TrafficLight> onYellow = (light) -> {...};
Consumer<TrafficLight> onGreen = (light) -> {...};

void onTrafficLightChange(TrafficLight light) {
  switch (light) {
    case TrafficLight.RED:
       onRed(light);
       break;
       ...
  }
}

Using a Map we can eliminate the switch statement thus making maintenance easier. Like introducting "YellowBlink" or "off"

static Map<TrafficLight, Consumer<TrafficeLight>> lights = new HashMap<>();
static Consumer<TrafficeLight> brokenLight = (light) -> {...};
static {
  lights.put(TrafficLight.RED, (light) -> {...});
  lights.put(TrafficLight.YELLOW, (light) -> {...});
  lights.put(TrafficLight.GREEN, (light) -> {...});
}

void onTrafficLightChange(TrafficLight light) {
  Consumer<TrafficeLight> c = lights.getOrDefault(light, () -> brokenLight);
  c.apply(light);
}

Since in this example TrafficLight is an Enum we have another option: Move the method to the Enum. You will never miss an update to the options.

public enum TrafficLight {
  RED {
    @Override
    public void onColor() {
      ...
    }
  },

  YELLOW {
    @Override
    public void onColor() {
      ...
    }
  },
  ...;

  public abstract void onColor(/* add params as needed*/);
}

void onTrafficLightChange(TrafficLight light) {
    light.onColor();
}

It all depends

As in writing, a style guide isn't set in stone but a collection of "This works for me".

So: YMMV

Update

Feedback on twitter by K.H. Marbaise pointed out a number of clarifications. While all examples will work with JDK 8, later versions allow better code:

  • In JDK16 or later replace .collect(Collectors.toList() with .toList() to return an immutable list
  • Update the Pair constructor from using Object to its generics (updated above) and don't allow null values (all JDKs)
  • In JDK17 replace the generic Pair class with a Java record
  • various typos, fixed

Posted by on 08 January 2023 | Comments (0) | categories: Development Java

Comments

  1. No comments yet, be the first to comment