New Inspection: Inform about Boolean Parameters

[saeltz]

Boolean parameters can easily lead to bugs. Especially if there are more than one.
This inspection informs about them, so developers can take a wise decision.

This has already been said by famous Robert C. Martin in Clean Code.

The formatting is part of #643 but this PR is based on the other to have working formatting.

[sksamuel]

Lgtm. @mccartney you ok to merge ?

[mccartney]

Do we have support for inspections being disabled by default?
While I can see this might be useful in some cases, but my hunch is that most folks would see it weird to discourage Boolean parameters to methods. The suggestion to use two methods instead (i.e. something similar to overloading) seems like an overkill. What if there were 2 or 3 Boolean parameters - would you suggest to have 4 or 8 methods instead?

Anyway - I don't mind adding this inspection. I'd only seek for it not being on by default (which I am not sure if we have support for).

[saeltz]

Do we have support for inspections being disabled by default?

Not really but the default list of disabled inspections could be changed to include this inspection. But that's not the nicest (but easy) solution.

What if there were 2 or 3 Boolean parameters - would you suggest to have 4 or 8 methods instead?

Robert Martin argues that if the method takes a boolean parameter than it most probably does two things based on an if. Then it should better be two methods.
If you have multiple booleans, the code becomes even more troublesome because it's easy to mix booleans at call-site, e.g.

methodName(x, true, false)
vs.
methodName(x, false, true)

This becomes easily unnoticeable without enforcing named parameters. (I haven't found a way to add that as an inspection.)
If the multiple booleans belong to a model that is handled in the method, one could argue to create a case class for the model and pass that instead, e.g.

final case class Model(x: X, isActive: Boolean, hasPermission: Boolean)
def methodName(model: Model) = ???

I'd only seek for it not being on by default

I specifically added this inspection on INFO level only, so it doesn't stop compilation. Same as with object name recommendations.

[mccartney]

Makes sense. Thanks for the context and the explanation. I think we are good to go.

Also filing #646 to simplify things.

All in all - I am happy to see your contributions. I am pretty sure Sam is happy too.

[sksamuel]

I am happy :)

…

On Mon, 2 May 2022 at 14:56, Greg Oledzki ***@***.***> wrote: Merged #644 <#644> into master. — Reply to this email directly, view it on GitHub <#644 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFVSGSS5AFLSUKIXBJGLYTVIAXPTANCNFSM5UMF3IKA> . You are receiving this because you commented.Message ID: ***@***.***>

[saeltz]

Could we maybe release a new minor version of scapegoat including this new inspection?