1

So as my the title says, SonarQube is complaining whenever you use 

list.size() == 0

or 

list.size > 0

However I started changing to isEmpty() and !is.Empty() and noticed the code becomes way less readable and had to change it back, therefore I don't think it's better to use the isEmpty() method as you have to read the condition very attentively instead of just seeing what the if clause does...

So I kind of totally disagree with SonarQube and my deciding argument is that using isEmpty is makes the code much less readable. Do you guys see any real advantages of using the isEmpty instead of checking the size when using this with Java collections which would outweigh the increased readability when not using it?

Improve this question
3
  • 1
    See this answer to a similar question on StackOverflow: stackoverflow.com/a/11152624/18938
    – Eric King
    Commented Oct 2, 2016 at 15:38
  • @EricKing so you kind of agree that we should always do it for performance instead of readability, correct?
    – Arturas M
    Commented Oct 2, 2016 at 15:47
  • 1
    Well, no... I actually think that isEmpty() is more readable, too. But that's subjective. I feel that if the readability isn't compromised too much, picking the more performant option is better.
    – Eric King
    Commented Oct 2, 2016 at 15:50

2 Answers 2

Reset to default
8

You should check the justification SonarQube gives as to why it makes the suggestion. I would bet that the primary reason is for performance, not readability.

If you don't like the readability of !collection.isEmpty(), then you could always use collection.isEmpty() == false.

Improve this answer
5
  • Well you are sort of right about that, SonarQube does state that the performance is expected better, I kind of just read the beginning of the description before, which stated: "makes the code more readable and can be more performant.", and with the readability I can't agree as it obviously isn't and I thought "can be" doesn't sound strong enough, but yeah, it does later say that: "The time complexity of any isEmpty() method implementation should be O(1) whereas some implementations of size() can be O(n)"
    – Arturas M
    Commented Oct 2, 2016 at 15:46
  • 3
    Some people (me included) would say that isEmpty() is inherently more readable than size == 0, since it more clearly implies the intent of the check. You're not really interested in the size, you're interested in whether the list is empty or not.
    – Eric King
    Commented Oct 2, 2016 at 15:49
  • @ArturasM: when you drive your car and run out of gas, which one would you be more likely to say: "Damn, my tank is empty!" or "Damn, the resulting value yielded by the act of measuring the size of the liquid in my tank equals the integral number zero!"?
    – Jörg W Mittag
    Commented Oct 3, 2016 at 1:08
  • @JörgWMittag That might be a good example, but totally unsuitable for this case. In this case a more appropriate example would be: we want to pick soldiers for a mission, we are calling them all from one tent, but we don't need them all and we need some who meet some criteria, so we pick them according to a criteria. And then we check our situation - how many soldiers we have that are eligible for this - more than one? (soldiers > 1), is it just one? (soldiers == 1) then we will keep that one but search in other tents, or is it 0? (soldiers == 0), and we take a look at those options and see!
    – Arturas M
    Commented Oct 3, 2016 at 14:11
  • @JörgWMittag For a tank isEmpty() might be OK if you just need to know if it's empty, but if you have more possible conditions in other cases where you are checking ranges and things like that, the isEmpty() is butchering the visibility, because you are basically seeing numbers and then you have to carefully read isEmpty() in order to switch your mode and to understand that it's the condition when the value is most likely to be 0. But Sonar doesn't have an exception if you have multiple cases, maybe they can improve that in the future versions?
    – Arturas M
    Commented Oct 3, 2016 at 14:16
4

list.size() == 0 is objectively less readable, because you are adding burden on the reader. If there's a problem in this area of code and a future maintainer is reading it, they will be forced to ask:

  • Is the size known at the start of the algorithm?
  • Is size() O(1) or O(n) for this data structure?
  • If it is O(n), has n recently become excessively large for this data structure?
  • If all these potential problems turned out not to be problems, did the original author actually carefully consider these and find them unproblematic, or did he or she just get lucky by just choosing to use size() == 0?

Readability isn't just about what you think looks nicer because you are more accustomed to it, it's about how difficult you make it for people to read and maintain your code later. When I see isEmpty(), I know I can safely skip a number of potential issues.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.