2

When I checked it in sonar, the result is:

Replace this lambda with a method reference.

It actually refers to this one: 

.filter(s -> aIDetailsDto.getResult().getIdNo().equals(s))

My code below goes like this:

AIDetailsDto aIDetailsDto = aaaService
        .getDetailsByUserId(userId)
    if (!ObjectUtils.isEmpty(aIDetailsDto)) {
      List<String> kvpValues = callService.getKVPCodes(NewConstants.REMOVED)
          .stream()
          .filter(s -> aIDetailsDto.getResult().getIdNo().equals(s))

I tried to change it however I get an error. Does anyone has an idea how to change it?

newbie
  • 101
  • 2
  • 4
  • 8
  • 1
    `s -> aIDetailsDto.getResult().getIdNo().equals(s)` could be replaced with `aIDetailsDto.getResult().getIdNo()::equals`, but this here makes it less readable IMO, I would suppress or ignore the sonar warning here – Eugene Jul 18 '18 at 09:26
  • @Eugene I think readability has to do with getting used to it. To me, using `::equal` is totally readable to me, because I'm used to it. My colleague, on the other hand, screams at me whenever I used *any* lambda, because he finds it terribly hard to read. – Jai Jul 18 '18 at 09:28
  • 1
    @Jai no no, I do prefer method references, but just not in *this* case – Eugene Jul 18 '18 at 09:29
  • What can I do to convert to method reference, if the expression is like this :- `s -> aIDetailsDto.getResult().getIdNo().equals(s.getIdNo())` – Sanal S Feb 26 '20 at 08:39

3 Answers3

2

You should use .filter(aIDetailsDto.getResult().getIdNo()::equals).

Jai
  • 8,165
  • 2
  • 21
  • 52
1

You can replace .filter(s -> aIDetailsDto.getResult().getIdNo().equals(s)) with .filter(aIDetailsDto.getResult().getIdNo()::equals), but you have to be aware of the semantic difference.

The lambda expression will evaluate the term aIDetailsDto.getResult().getIdNo() every time the predicate function is evaluated, while the method reference will evaluate it when the Predicate instance is created and capture the result value, to invoke equals on the same object on each subsequent predicate evaluation. See also “What is the equivalent lambda expression for System.out::println”.

If the expression aIDetailsDto.getResult().getIdNo() is supposed to evaluate to the same result each time, it makes no difference and the method reference might be more efficient due to not evaluating it repeatedly.

Note also the alternative .filter(Predicate.isEqual(aIDetailsDto.getResult().getIdNo())), which will also evaluate the argument expression only once, but is the only variant handling the case that it evaluated to null, by then accepting onlynull elements (see Predicate.isEqual(…)). However, when this value is supposed to never be null, the throwing behavior of the other variants is preferable.

Holger
  • 285,553
  • 42
  • 434
  • 765
0

Just think of the fact that you are calling this aIDetailsDto.getResult().getIdNo() for every single s, what would make it better and also make sonar (and you) happy, is to move it up a bit (and call it only once):

String idNo = aIDetailsDto.getResult().getIdNo();
... stream()
    .filter(idNo::equals)
Eugene
  • 117,005
  • 15
  • 201
  • 306