5

I have a combination of querying a database with jooq and post processing the result with the streams. However I feel that my code is not very readable and not concise enough. How can I improve my code in ways of better expressing my intent.

sql
    .select(field("USER_NAME", String.class))
    .from(table("CWD_USER"))
    .fetch()
    .stream()
    .map(f -> f.getValue(field("USER_NAME", String.class)))
    .collect(Collectors.groupingBy(s -> StringUtils.split(s, "-")[0], Collectors.counting()))
    .entrySet().stream()
    .sorted(new java.util.Comparator<Entry<String, Long>>() {
        @Override
        public int compare(Entry<String, Long> o1,
                Entry<String, Long> o2) {
            return o2.getValue().compareTo(o1.getValue());
        }
    })
    .forEach(e -> System.out.println(String.format("%13s: %3d", e.getKey(), e.getValue())));

First I have problems with the multiple streaming. I first stream the result from jooq then I stream the collected map. Also the comparator seems way to prominent. Sure I could make a class out of it, but maybe there is another solution.

Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334
Angelo.Hannes
  • 1,729
  • 1
  • 18
  • 46
  • 3
    Even if you don’t know the existence of [`Map.Entry.comparingByValue(…)`](http://docs.oracle.com/javase/8/docs/api/java/util/Map.Entry.html#comparingByValue--), I don’t get why you think that you have to use an inner class for the `Comparator`, as you have already shown to know lambda expressions at the other places in your code. – Holger Sep 23 '15 at 08:31
  • 1
    Just for reference, the corresponding lambda would have been `.sorted( (o1, o2) -> o2.getValue().compareTo( o1.getValue() ) )`. Thanks for pointing that out @Holger. – Angelo.Hannes Sep 23 '15 at 08:38
  • `I first stream the result from jooq then I stream the collected map` - no. You stream the results from jooq and thats it. There are no additional streams shown in your example, thats one single stream with a few intermediate operations and exactly one terminal operation. Thats how streams work. If you want to "avoid" the core functionality of streams you need to use classic foreach/for/while - loops. But honestly : that'd be a downgrade ... streams are very concise and beautiful - your own code demonstrates this. The same functionality would need at least 100 lines of code without streams. – specializt Sep 23 '15 at 08:39
  • 2
    @specializt: look closer. There is a `collect` in between. That’s the terminal operation for the first stream and `entrySet().stream()` creates a new stream. Not that it matters, as that’s unavoidable here. – Holger Sep 23 '15 at 09:01
  • 1
    The intermediate map is unavoidable, especially with the sorting that follows, because those operations must complete before the next step in the pipeline can be executed, otherwise it wouldn't be able to know if it had already found the *min*-element – the8472 Sep 23 '15 at 11:51

2 Answers2

5

I cannot say about JOOQ part, but the Stream API part looks fine. You have to collect intermediately to know the counts prior to sorting. Note that such comparator is already implemented in JDK: it's Map.Entry.comparingByValue(). You can use it (add Comparator.reverseOrder() parameter to sort in reverse order):

sql
    .select(field("USER_NAME", String.class))
    .from(table("CWD_USER"))
    .fetch()
    .stream()
    .map(f -> f.getValue(field("USER_NAME", String.class)))
    .collect(Collectors.groupingBy(s -> StringUtils.split(s, "-")[0], Collectors.counting()))
    .entrySet().stream()
    .sorted(Map.Entry.comparingByValue(Comparator.reverseOrder()))
    .forEach(e -> System.out.println(String.format("%13s: %3d", e.getKey(), e.getValue())));
Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334
2

Unless this is a drastically simplified version of a more complex query, I'd move all logic to SQL. The equivalent SQL query (using the Oracle dialect) is:

SELECT PREFIX, COUNT(*)
FROM (
  SELECT SUBSTR(USER_NAME, 1, INSTR(USER_NAME, '-') - 1) AS PREFIX
  FROM CWD_USER
) T
GROUP BY PREFIX
ORDER BY COUNT(*)

Or, with jOOQ:

sql.select(field("PREFIX", String.class), count())
   .from(
     select(substring(
       field("USER_NAME", String.class), 
       inline(1), 
       position(field("USER_NAME", String.class), inline("-")).sub(inline(1))
     ).as("PREFIX"))
     .from(table("CWD_USER"))
   )
   .groupBy(field("PREFIX", String.class))
   .orderBy(count())
   .fetch();
Lukas Eder
  • 211,314
  • 129
  • 689
  • 1,509