I propose to add in S6611 why NoSuchElementException is better than NullPointerException

SonarQube Server / Community Build
1

I propose to add an explanation to the Compliant solution of S6611 “Map values should be accessed safely” why NoSuchElementException is better than NullPointerException.

2

Hi,

Welcome to the community!

What explanation do you propose?

 
Ann

3

Personally, I see the only advantage in the fact that the error message contains the key for which the value was not obtained. Perhaps you meant something else.

4

Hi,

I suspect we’re not quite connecting on meaning.

Do you mean that you suggest we come up with and add an explanation to the compliant solution?

I guess you’re saying the issue message isn’t helpful enough?

 
Thx,
Ann

5

Yes, that’s right. I suggest adding a rationale - why you are suggesting replacing NullPointerException with NoSuchElementException.

1 Like
8

Hi,

Thanks for the clarification. Just one more question: what’s your version of SonarQube? You can find it in the page footer if you’re not sure.

 
Thx,
Ann

9

version is Community Edition Version 10.4 (build 87286)

1 Like
12

Hi @kirinalexdev ,

I would argue that the rule description could be simplified and does not hit on-spot the true reason why we should use getValue(...) instead of get(...)!!. I created a ticket here to refine the rule description. The refined description should resolve your question, as NoSuchElementException is a mere result from the API contract of getValue. You should use that function even if the API designers would have decided for a NullPointerException - which they didn’t, because NoSuchElementException is the more descriptive exception for this use-case.

The problem with the current description:

I think it’s not correct to say:

In Kotlin, it’s not usually convenient to operate with nullable types, so developers usually try to avoid them or convert them to non-nullable types.

This is not what null-safety means. Instead, it means to explicitly state whether a type can be null, to prevent unexpected NPEs during runtime.

The reason we should prefer getValue(...) over get(...)!! is that get and getValue have different use-cases. We should always use an API as intended and in an idiomatic way. If there is an API function for a specific use-case, we should use that function instead of recreating its behavior.

The use-cases here are:

  • getValue() - Use when we expect a value to be present in the map by design. If not present, throw an exception. This is likely to be an error.
  • get() - Use when the value might or might not be present in the
    map by design. If it’s not present, return null to handle this as a special case.
1 Like