We are the Dev Teams of
  • brands
  • ebay_main
  • ebay
  • mobile
<
>
BLOG

Clean Streaming Code

by Lars Opitz
in Tutorials

Clean Streaming Code

Streaming is an awesome tool. It helps to separate concerns (e.g. iteration, filtering, execution), to avoid state and thus improves thread safety. Furthermore, we can express the same functionality with less code, but it's hard to read and to test. 

Really?

In a recent project we introduced Java 8 streaming to legacy code and converted a lot of loops. Although I’m used to writing code that makes use of Java 8 streaming API, I realized how hard it is to read similar code written by others (and how hard my code might be to read). Furthermore, there were bugs in the code although the tests were all green and the coverage 100%. These tests were quite complicated as they had to test the whole method including iteration, filtering and execution: a lot of concerns.

The legacy code was as follows:

private void addLinksToTarget(List<Combination> links, 
        Combination combination, String caseDesc) {
    for (Iterator<Combination> linksIterator = links.iterator(); links.hasNext()
            && combination.getTargets().size() < PLACES_PER_KEYWORD;) {
        Combination link = linkIterator.next();
        if (!link.getSegment().equals(combination.getSegment())) {
            continue;
        }

        Rating linkLimit = link.getRatingByType(RatingType.AVAILABLE_PLACES);

        if (linkLimit.getValue() <= 0) {
            linkIterator.remove();
            continue;
        }

        // avoid duplicate urls in the same footer
        String sourceUrl = urlFormatter.format(combination);
        String targetUrl = urlFormatter.format(link);
        if (targetUrl.equals(sourceUrl)) {
            continue;
        }
        boolean alreadyContained = false;
        for (String footerPart : combination.getTargets()) {
            if (StringUtils.containsIgnoreCase(footerPart, targetUrl)) {
                alreadyContained = true;
                break;
            }
        }
        if (alreadyContained) {
            continue;
        }

        String target = link.getLabel() 
                        + ">" + targetUrl 
                        + ">" + link.getPriority() 
                        + ">" + caseDesc;
        combination.getTargets().add(target);
        linkLimit.setValue(linkLimit.getValue() - 1);
    }
}

A colleague undertook the challenge to transforme the original version to the following using Java 8 streaming.

private void addLinksToTarget(List<Combination> links, 
        Combination combination, String caseDesc) {
    links.stream()
         .filter(link -> link.getSegment().equals(combination.getSegment()))
         .filter(link -> link.getRatingByType(AVAILABLE_PLACES)
                         .orElse(new Rating(AVAILABLE_PLACES, 0)).getValue() > 0)
         .filter(link -> !isSelfReference(combination, link))
         .filter(link -> !isAlreadyContainedInTargets(link, combination))
         .limit(linksPerCombination - combination.getTargets().size())
         .forEach(link -> {
                String targetUrl = urlFormatter.format(placeable);
                String target = link.getLabel() 
                                + ">" + targetUrl 
                                + ">" + link.getPriority() 
                                + ">" + caseDesc;
                combination.getTargets().add(target);
                Rating linkLimit = link.getRatingByType(AVAILABLE_PLACES).get();
                linkLimit.setValue(linkLimit.getValue() - 1);
            });
    links.removeIf(link -> {
        Optional<Rating> rating = link.getRatingByType(AVAILABLE_PLACES);
        return !rating.isPresent() || rating.get().getValue() <= 0;
    });
}

This code now looks much better, but it’s still hard to read – at least was for me as an outsider. So I started refactoring the code to a more readable form. I created predicates containing the filter logic and give them meaningful names and extracted the business logic executed in forEach to a consumer.

private void addLinksToTarget(List<Combination> links, 
        Combination combination, String caseDesc) {
    Helper helper = Helper.of(combination, urlFormatter, caseDesc);
    links.stream()
         .filter(helper.sameSegments)
         .filter(helper.hasLinkPlacesLeft)
         .filter(helper.notSelfReference)
         .filter(helper.linkNotContained)
         .limit(helper.maxAvailableLinks)
         .forEach(helper.addLinkToTargetWithCaseDescription);
    links.removeIf(helper.noLinkPlacesLeft);
}

Every condition and the consumer are now easily testable (and isolated), and possible bugs can be found faster. But more importantly: I can easily understand what this code block is intended to do, even if I wasn’t the author.

Conclusion

We learned great things from Uncle Bob like moving blocks of logic into self-describing methods or not being done before the code is refactored to be well readable by others (including my future self in half a year). Apparently we sometimes forget these lessons when it comes to new features like the streaming API of Java 8. Maybe it’s because we’re so excited to have them and think it’s all much clearer now – or simply because we need to gather some more experience.

Clean Code, java, Java, clean code, programming

?>