Remove the literal "false" boolean value-CodePudding

 public List<LawOfficeDetailEntity> getLawOfficeByManagementUnitId(Long managementUnitId, Boolean statusless) {
        List<LawOfficeDetailEntity> entities = lawOfficeDetailRepository.findByManagementUnitType(String.valueOf(managementUnitId));
        return (statusless == null || statusless == false) ? entities.stream().filter(office -> office.getValidityStatus() == 1).collect(Collectors.toList()) : entities;
    }

I have "remove the literal "false" boolean value" sonar error on statusless == false. How can i fix it?

CodePudding user response：

As you use Boolean and not the primitive type boolean, you should not compare it with ==. What you can do is: statusless == null || !statusless.booleanValue()

CodePudding user response：

Given that this question is about a linting tool, it is therefore strictly about proper code style and hygiene, and therefore the question is a bit of a tricky one: Yes, there is a way to 'avoid the sonar warning', but that is like handing a bazooka to a child: The one and only reason to use sonar is to write proper code, and the fastest, simplest way to 'avoid the sonar warning' makes your code worse.

So, I best instead explain what's truly wrong with this code instead!

The underlying problem is that you've turned a boolean into a ternary state: It can be true, false, OR null, and this makes sense only in the SQL sense: That null is indicative of unknown, not applicable, or not yet set. In all 3 such cases, treating null as false is just plain wrong.

Whatever makes statusless be null? Fix that. Make it not be that. If it's a thing you get from another system, fix it in the other system (Example: If this is a nullable BOOLEAN db column, give it a non-null constraint and a FALSE default value), or if you can't fix it in the other system, fix it immediately once the data arrives at 'your side' of the code.

Once you've done all that, the possibility for statusless to be null goes away.

NB: Linting tools like sonar are a really bad idea unless you fully understand each and every warning it gives you. A linting tool tends to find medium to low-level problems with code, and can rarely detect high-level, more important issues with code, as is the case here.

If somehow you want to ignore this advice as you have determined that carrying on with a lying variable ('boolean', even 'Boolean', rather strongly suggests either 2 states, or perhaps 2 states the ability to remain unset, but certainly not 'three states') is correct anyway, you can simply replace statusless == null || statusless == false with: !Boolean.TRUE.equals(statusless). Boolean.TRUE reports false unless you pass a true value - so whether statusless is false or null - either way, TRUE.equals(that) will return false. Flip that false into true and voila.

You can't write !statusless because that will throw an NPE if statusless is null.

But wait! Alternate interpretation!

Perhaps that Boolean really is supposed to represent the borderline acceptable notion of 'this value is either true, or false, or hasn't been set yet / does not apply to this situation', in which case the NPE is precisely what you want here: You can't determine whether to filter by validity status or not, and yet you must here, therefore this code cannot continue and the error lies in the caller having set up invalid state before calling this method. In that case, you really just want return !statusless ? ... : ...;