CodeSOD: Swimming Downstream

When Java added their streams API, they brought the power and richness of functional programming styles to the JVM, if we ignore all the other non-Java JVM languages that already did this. Snark aside, streams were a great addition to the language, especially if we want to use them absolutely wrong.

Like this code Miles found.

See, every object in the application needs to have a unique identifier. So, for every object, there's a method much like this one:

/** * Get next priceId * * @return next priceId */ public String createPriceId() { List<String> ids = this.prices.stream().map(m -> m.getOfferPriceId()).collect(Collectors.toList()); for (Integer i = 0; i < ids.size(); i++) { ids.set(i, ids.get(i).split("PR")[1]); } try { List<Integer> intIds = ids.stream().map(id -> Integer.parseInt(id)).collect(Collectors.toList()); Integer max = intIds.stream().mapToInt(id -> id).max().orElse(0); return "PR" + (max + 1); } catch (Exception e) { return "PR" + 1; } }

The point of a stream is that you can build a processing pipeline: starting with a list, you can perform a series of operations but only touch each item in the stream once. That, of course, isn't what we do here.

First, we map the prices to extract the offerPriceId and convert it into a list. Now, this list is a set of strings, so we iterate across that list of IDs, to break the "PR" prefix off. Then, we'll map that list of IDs again, to parse the strings into integers. Then, we'll cycle across that new list one more time, to find the max value. Then we can return a new ID.

And if anything goes wrong in this process, we won't complain. We just return an ID that's likely incorrect- "PR1". That'll probably cause an error later, right? They can deal with it then.

Everything here is just wrong. This is the wrong way to use streams- the whole point is this could have been a single chain of function calls that only needed to iterate across the input data once. It's also the wrong way to handle exceptions. And it's also the wrong way to generate IDs.

Worse, a largely copy/pasted version of this code, with the names and prefixes changed, exists in nearly every model class. And these are database model classes, according to Miles, so one has to wonder if there might be a better way to generate IDs"

[Advertisement] ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out!