Hackerrank Sparse Arrays in Java 8

I have recently started trying Java 8 and I am trying to solve problems as much possible using Java 8 lingo.

I wrote the code for the Sparse Arrays problem on HackerRank and needed to know a few pointers regarding my code:

• Is it the right way to code in Java 8?

• How much more efficient is it than legacy Java programs?

• Are there any improvements or misconceptions regarding the association of this code to Java 8?

public class SparseArrays {

    public static void main(String[] args) throws Exception {

        Scanner sc = new Scanner(System.in);

        int N = sc.nextInt();
        List corpus = new ArrayList();
        IntStream.range(0, N).forEach(n -> corpus.add(sc.next()));

        int Q = sc.nextInt();
        List queries = new ArrayList();
        IntStream.range(0, Q).forEach(n -> queries.add(sc.next()));

        queries.stream().forEach(query -> System.out.println(corpus.stream().filter(p -> p.equals(query)).count()));
    }
}

Your code uses streams quite neatly and well. The algorithm employed is not necessarily the best, but the logic is reasonably clear.

It's common with streams to use vertical white-space to make the steps in code more readable. For example, the line:

queries.stream().forEach(query -> System.out.println(corpus.stream().filter(p -> p.equals(query)).count()));

could be re-formatted as:

queries.stream()
       .forEach(query -> System.out.println(
              corpus.stream()
                    .filter(p -> p.equals(query))
                    .count()
               ));

Now, while dealing with that stream, it's not really streaming things "right".... The System.out.println is out-of-place. You should be mapping the query in to a count in one step, and then outputting the count in a different step....

queries.stream()
       .mapToInt(query -> corpus.stream()
                    .filter(p -> p.equals(query))
                    .count()
               )
       .forEach(System.out::println);

Now, there are ways to improve that performance still, for example, lots of println statements could be grouped in to just one print...

String output = queries.stream()
       .mapToInt(query -> corpus.stream()
                    .filter(p -> p.equals(query))
                    .count()
               )
        .mapToObj(String::valueOf)
        .collect(Collectors.joining("\n"));
System.out.println(output);

Finally, note that you are doing an \$O(n)\$ operation for each query. You should consider a way of improving that time-cost by front-loading the creation of a Map when loading up the words will improve the performance of querying things.

For example, consider this:

IntStream.range(0, N).forEach(n -> corpus.add(sc.next()));

First up, that inner part n -> corpus.add(sc.next()) is doint a "side-effect" operation that can be avoided. Functional-programming purists try to avoid side-effects in streasm as much as possible. SOmetimes it is unavoidable, but you could easily re-write that line as:

List corpus = IntStream.range(0, N)
      .mapToObj(n -> sc.next())
      .collect(Collectors.toList());

That adds each word to a list. How about doing it this way:

Map> data = IntStream.range(0, N)
         .mapToObj(i -> sc.next())
         .collect(Collectors.groupingBy(v -> v));

That code takes each input word, and returns a Map keyed by each unique word, and each word's value is a list containing each instance of that word in the source.

Now, you can use the length (size) of the value list to return your data with:

String output = queries.stream()
        .mapToInt(query -> data.get(query).size())
        .mapToObj(String::valueOf)
        .collect(Collectors.joining("\n"));
System.out.println(output);

StackExchange Code Review Q#154853, answer score: 4