java - Return from map after filter better way

Question

hello I have this function setanimalhappy to my Zoo program, but I don't think it's well written i think the code after return is addition, it is useless

(return personRepository.save(person).getRecordList().parallelStream() .filter(record1 -> record1.getId().equals(animalRecord.getId())) .findFirst().orElseThrow(RecordException::new); }) .orElseThrow(PersonException::new);)

get person -> find record in record list -> update value save to record list -> return edited record

how can i return record1 from map to outside function or make this code better ?

@PutMapping("/{id}/animalhappy")
@ResponseBody
public Record SetAnimalHappy(@PathVariable String id, @RequestBody Record animalRecord) {

    return personRepository.findById(id)
            .map(person -> {
                person.getRecordList().parallelStream()
                        .filter(record1 -> record1.getId().equals(animalRecord.getId()))
                        .findFirst()
                        .map(record1 -> {
                            record1.getAnimalStatus().setHappy(true);
                                    return record1;
                                }
                        ).orElseThrow(RecordException::new);

                personRepository.save(person);

                return personRepository.save(person).getRecordList().parallelStream()
                        .filter(record1 -> record1.getId().equals(animalRecord.getId()))
                        .findFirst().orElseThrow(RecordException::new);
            })
            .orElseThrow(PersonException::new);
}

question from:https://stackoverflow.com/questions/65917773/return-from-map-after-filter-better-way

Answer 1

You don't seem to need to #map here, as if the Optionals are empty, you want exceptions to be thrown:

// this should probably live in the service layer instead of the controller layer:
public Record setAnimalHappy(String id, Record animalRecord) {
    Person person = personRepository
        .findById(id)
        .orElseThrow(PersonException::new);
    // the following might be replaced by recordService.findById(animalRecord.getId()),
    // while making sure the returned record belongs to `person`:
    Record record = person
        .getRecordList()
        .stream() // don't use parallel streams (unless you know what you're doing)
        .filter(record -> record.getId().equals(animalRecord.getId()))
        .findFirst()
        .orElseThrow(RecordException::new);
    // not 100% sure the 2 following lines behave the way you'd expect:
    record.getAnimalStatus().setHappy(true);
    personRepository.save(person);
    return record;
}

Business logic apart, something along those lines might be arguably more readable.

It's easy to lose yourself into callbacks when entering the functional programing world. One rule I tend to follow, is "don't use -> {" (multiline lambda expressions): either reorganise your code (like the above example), or use additional functions to improve readability.

Interesting related read: https://dzone.com/articles/functional-programming-patterns-with-java-8.