CodeSOD: Revenge of the Stream
It's weird to call Java's streams a "new" feature at this point, but given Java's prevalence in the "enterprise" space, it's not surprising that people are still learning how to incorporate them into their software. We've seen bad uses of streams before, notably thanks to Frenk, and his disciple Grenk.
Well, one of Antonio's other co-workers "learned" their lesson from Frenk and Grenk. Well, they learned a lesson, anyway. That lesson was "don't, under any circumstances, use streams".
Unfortunately, they were so against streams, they also forgot about basic things, like how lists and for-loops work, and created this:
private List<Integer> createListOfDays(String monthAndYear) { List<Integer> daysToRet = new ArrayList<>(); Integer daysInMonth = DateUtils.daysInMonth(monthAndYear); for (Integer i = 1; i <= daysInMonth; i++) { daysToRet.add(i); } Set<Integer> dedupeCustomers = new LinkedHashSet<>(daysToRet); daysToRet.clear(); daysToRet.addAll(dedupeCustomers); Collections.sort(daysToRet); return daysToRet; }
The goal of this method is, given a month, it returns a list of every day in that month, from 1...31, or whatever is appropriate. The date-handling is all taken care of by daysInMonth, which means this is the rare date-handling code where the date-handling isn't the WTF.
No, the goal here is simply to populate an array with numbers in order, which the for-loop handles perfectly well. It's there, it's done, it's an entirely acceptable solution. Just return right after the for loop, and there's no problem at all with this code. You could just stop.
But no, we need to dedupeCustomers, which oh no, they just copied and pasted this code from somewhere else. In this case, to remove duplicates, they use a Set, or specifically a LinkedHashSet, which is one of the many set implementations Java offers as a built-in. A hash set, doesn't retain order, in contrast to something like a TreeSet, which does.
I bring that ordering thing up, because we started with our list in sorted order, with no duplicates. We added it to a set, destroying the order and removing the duplicates we don't have. Then, we clear the original list, jam the unsorted data back into it, and then have to sort it again.
This code made Antonio angry, and dealing with Frenk's unholy streams also made him angry, so Antonio decided to not only fix this method, but use it to demonstrate a stream one-liner which wasn't a disaster:
private List<Integer> createListOfDays(String monthAndYear) { return IntStream.rangeClosed(1, DateUtils.daysInMonth(monthAndYear)).collect(ArrayList::new, List::add, List::addAll); }[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!