CODEWITHSUNDEEP
javajava-stream
Sreehari Puliyakkot
Sreehari Puliyakkot

Reputation: 736

Java stream elegant solution to avoid looping multiple times

class Note{
    private text;
    ..
    private int score = 0;
}

class Project{
    ...
    List<Note> notes;
    private int score = 0;
}

Project score is derived by project properties + sum of note scores.

First I'm updating and replacing all notes in the project. Then iterating again to sum the note score.

 project.notes(project.notes()
        .stream()
        .map(this::updateNote)
        .collect(Collectors.toList()));

 project.score(project.notes()
                .stream()
                .mapToInt(n->n.score())
                .sum());

private Note updateNote(Note note){
    note.score(....);
    return note;
}

Somehow I feel this is not right. Is there an elegant solution to avoid looping twice?

Upvotes: 2

Views: 564

Answers (2)

rzwitserloot
rzwitserloot

Reputation: 103893

Performing a side-effect-ful method in a map operation like this is suspect no matter how you want to slice this puzzle, it's always going to look a bit weird.

You're abusing map here: You map objects to itself, but in so doing, cause a side effect. well, in for a penny, in for a pound, I guess - that is the suspect thing here, but the code works (it's just bad style). Note also that passing the collected list to project.notes does nothing, just project.notes().stream().map(this::updateNote).collect(Collectors.toList()) and letting the collection be lost to the either already 'works'. The only point of collecting is merely to force the stream to actually iterate (map doesn't cause iteration, it merely says: When you start iterating, map as-you-go).

So:

project.notes()
  .stream()
  .map(this::updateNote)) // no-op streamwise. Just making the side effect happen
  .mapToInt(this::score)
  .sum();

is all you need - but it's.. still a bit stinky. If instead of updateNote, note was immutable and there is a calculateScore method, you could do:

project.notes()
    .stream()
    .mapToInt(this::calculateScore)
    .sum()

Here, calculateScore doesn't change anything about a Note object, it merely.. calculates the score and returns it, without changing any fields.

EDIT: I forgot a 'stream' in stream, and added a clarification.

Upvotes: 1

Nowhere Man
Nowhere Man

Reputation: 19575

You may get rid of looping twice by accumulating the sum in an AtomicInteger, but this would result in replacing the method reference this::updateNote with a lambda:

AtomicInteger sum = new AtomicInteger(0);

project.notes(project.notes()
        .stream()
        .map(note -> {
            Note updated = updateNote(note);
            sum.addAndGet(updated.getScore());
            return updated;
        })
        .collect(Collectors.toList()));

project.score(sum.intValue());

Upvotes: 1

Related Questions