I don’t see a problem with the first example, the third example is longer than the second one, which to me justifies being refactored into the first or the second. In terms of readability, in my opinion the first is the most readable, the second is less readable and the third is least readable.
If readability is subjective, then the S1612 rule should not exist at all. How else can you justify its existence, if not readability? The rule itself states:
Method/constructor references are more compact and readable than using lambdas, and are therefore preferred.
How can it claim that method references are more readable, if readability is subjective? And because this rule exists, that subjectivity is further proof that Sonarqube shouldn’t be so gung-ho with slapping warnings for it.
Additionally, the direct quote from Joshua Bloch’s summary is:
Where method references are shorter and clearer, use them; where
they aren’t, stick with lambdas.
Going by typical programming assumptions, a method reference should be both clearer and shorter for it to be considered a good choice.
I know this sounds a bit anal but obviously there is no way to objectively define what code is “clearer” programmatically, so I think the length of code is a good enough heuristic.
It is definitely a much better one than simply always suggesting the change, regardless of the length of code. Checking the length will result in less false positives than the current method.
As for “having such a long class name”, that class name was obviously an example. A more realistic one would be a class called
ValidationErrorResponse - in my opinion that name is not too long.
I will forever maintain that
.map(ValidationErrorResponse::extractValidationError) is less readable than
.map(e -> extractValidationError(e)). As you can see, I am facing this issue, even while having a reasonable class name. Yes, the
e -> part is fluff, but so is
ValidationErrorResponse::. Both have to be ignored while trying to discern what the code does, but the earlier is easier to ignore than the latter due to the length.
The length check is the obvious check. It’s better for Sonarqube to not report a minor violation of readability than fill the dashboard with false positives which have to be manually checked off. A code checking tool should never chastise its user for writing good code.
All in all, just because the check will not be 100% perfect does not mean it should not be implemented. I’m not advocating for a “Method reference should be replaced with lambda” rule here, I’m merely asking for Sonarqube not to punish me for writing clear, (most often) more readable code.
Adding an exception is kind of accepting that a long class name is fine.
Class name length has nothing to do with this rule. If long class names are not fine (which I actually agree with), then that should be a separate SonarQube rule.