CODEWITHSUNDEEP
javajava-stream
M Watson
M Watson

Reputation: 103

Nested Loop to Java Stream Puzzle

I'm new to streams in Java 8. I am struggling to figure out how to implement a nested for loop on lists using streams. One list contains messages, one of the fields in the message is the id of an author (not an obj reference, sadly). The other list is the authors, and one of the fields in the author is the author id. The code needs to find the matching author in the authors list for each message in the message list. The working Java code I have (which does not use streams or lambdas) is as follows, my question is how would I rewrite this using streams? I have tried a couple of things, I thought a flatMap and a filter would be needed, but I cannot seem to get it right.

public void showAuthors(List<Message> messages, List<Authors> authors) {
    String authorName=null;
    for (Message message : messages) {
        int authorId = message.getAuthorId();
        for (Authors author : authors) {
            if (author.getId() == authorId)
                authorName = author.getFullName();
                break;
        }
        System.out.println("Message id is " + message.getId() + 
                           " author is " + authorName);
    }
}

I've looked at some similar questions here, but I just can't seem to get it right.

Any help is appreciated!

=============================================

Update: The list of Authors has some null data for the Id fields. This required that I use filters with the stream operation to create the map of AuthorId,Name.

Upvotes: 1

Views: 194

Answers (4)

M Watson
M Watson

Reputation: 103

After trying some of the suggestions above, I quickly realized that the authors data has some objects with no Id, just names (apparently they serve a dual purpose...it's not my design!).

So, I wrote the following based on help/hints from the answers above:

public void showAuthors(List<Message> messages, List<Reference> authors) {
Map<Integer, String> authorsById = authors.stream()
    .filter(author -> !(author.getId() == null))
    .filter(name -> !(name.getFullName() == null))
    .collect(Collectors.toMap(Reference::getId, Reference::getFullName));

messages.forEach(
    message -> System.out.println("Message id is " + message.getId() + 
              " Author is " + authorsById.get(message.getSenderId())));

Is there anything wrong with this solution?

Also, I'm new to stackoverflow (as I'm sure you could tell)...is there a way I can give all 3 of the responders something for helping? Is that what upvoting is for?

Meg

Upvotes: 2

Alexander Anikin
Alexander Anikin

Reputation: 1098

You don't need streams for messages processing, here forEach is more suitable. Nevertheless, streams are useful for authors list to map conversion.

You can use toMap with 3 arguments to avoid possible authors duplication in authors list (original code avoids this).

You can use Optional to avoid possible absence of authos, null id etc. - each time .map returns null, Optional becomes Optional::Empty, so further processing for it is skipped up to terminal operation, and ifPresence simply skips empty optionals.

Map<Integer, String> authorNames=authors.stream()
    .collect(Collectors.toMap(
        Author::getId,
        Author::getFullName,
        (oldValue, newValue) -> oldValue));

messages.forEach(
    m -> Optional.of(m)
        .map(Message::getAuthorId)
        .map(authorNames::get)
        .map(name -> "Message id is " + m.getId() + " author is " + name)
        .ifPresent(System.out::println)
);

Upvotes: 0

Ravindra Ranwala
Ravindra Ranwala

Reputation: 21124

In the first place to me your current logic is wrong. It breaks the inner loop if the condition is not satisfied for the first time and for all messages you get the previous author or some invalid results like so. 

Message id is 1 author is AuthOne
Message id is 2 author is AuthOne
Message id is 4 author is AuthOne

Here's the corrected one.

public static void showAuthors(List<Message> messages, List<Author> authors) {
    String authorName = null;
    for (Message message : messages) {
        int authorId = message.getAuthorId();
        for (Author author : authors) {
            if (author.getId() == authorId) {
                authorName = author.getFullName();
                break;
            }
        }
        System.out.println("Message id is " + message.getId() + " author is " + authorName);
    }
}

You have to break the loop only after the first Author is found. Here's the equivalent stream counterpart with linear time complexity.

Map<Integer, String> authorsById = authors.stream()
        .collect(Collectors.toMap(Author::getId, Author::getFullName));
Map<Integer, String> authorNameAgainstId = messages.stream()
        .collect(Collectors.toMap(Message::getId, m -> authorsById.get(m.getAuthorId())));

Upvotes: 1

Mạnh Quyết Nguyễn
Mạnh Quyết Nguyễn

Reputation: 18235

Since you're doing mxn comparison, so it's better to create a Map between authorId -> author to avoid O(n) lookup on authors:

Map<Integer, Author> authorMaps = authors.stream().collect(toMap(Author::getId, Function.identity()));

Now you can get the author for each message like this:

messages.stream().filter(m -> authorMaps.containsKey(m.getAuthorId()))
        .forEach(m -> 
            System.out.println("Message id is " + m.getId() + 
                           " author is " + authorMaps.get(m.getAuthorId()). getFullName());
        );

Upvotes: 1

Related Questions