CodeSOD: What a Stream

In Java 8, they added the Streams API. Coupled with lambdas, this means that developers can write the concise and expressive code traditionally oriented with functional programming. It's the best bits of Java blended with the best bits of Clojure! The good news, is that it allows you to write less code! The better news is that you can abuse it to write more code, if you're so inclined.

Antonio inherited some code written by "Frenk", who was thus inclined. Frenk wasn't particularly happy with their job, but were one of the "rockstar programmers" in the eyes of management, so Frenk was given the impossible-to-complete tasks and given complete freedom in the solution.

Frenk had a problem, though. Nothing Frenk was doing was actually all that impossible. If they solved everything with code that anyone else could understand, they wouldn't look like an amazing genius. So Frenk purposefully obfuscated every line of code, ignoring indentation, favoring one-character variable names, and generally trying to solve each problem in the most obtuse way possible.

Which yielded this.

 Resource[] r; //@Autowired ndr Map<File, InputStream> m = null; if (file != null){ m.put(file, new FileInputStream(file));}else{ m = Arrays.stream(r).collect(Collectors.toMap(x -> { try { return x.getFile(); }catch (Exception e) { throw new IllegalStateException(e);}}, x -> {try{return x.getInputStream();}catch (Exception e){throw new IllegalStateException(e);}}));}

As purposefully unreadable code, I'd say that Frenk fails. That's not to say that it isn't bad, but Frenk's attempts to make it unreadable" just make it annoying. I understand what the code does, but I'm just left wondering at why.

I can definitely say that this has never been tested in a case where the file variable is non-null, because that wouldn't work. Antonio confirms that their IDE was throwing up plenty of warnings about calling a method on a variable that was probably null, with the m.put(") line. It's nice that they half-way protect against nulls- one variable is checked, but the other isn't.

Frenk's real artistry is in employing streams to convert an array to a map. On its surface, it's not an objectively wrong approach- this is the kind of things streams are theoretically good at. Examine each element in the array, and apply a lambda that extracts the key and another lambda that extracts the value and put it into a map.

There are many real-world cases where I might use this exact technique. But in this case, Antonio refactored it to something a bit cleaner:

 Resource[] resources; //@Autowired again Map<File, InputStream> resourceMap = new HashMap<>(); if (file != null) resourceMap.put(file, new FileInputStream(file)); else for (Resource res : resources) resourceMap.put(res.getFile(), res.getInputStream());

Here, the iterative approach is much simpler, and the intent of the code is more clear. Just because you have a tool doesn't make it the right tool for the job. And before you wonder about the lack of exception handling- both the original block and the refactored version were already wrapped up in an exception handling block that can handle the IOException that failed access to the files would throw.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!