I’m wondering what’s wrong with an empty function ...

# detekt

[Ellen Spertus]

I’m wondering what’s wrong with an empty function block. I have a class with a stop() method that currently doesn’t do anything. I want the API to say that the stop() method should be called when it is no longer needed. It’s not the caller’s business whether any instructions are there or not. There may be in the future. Or is this a case of YAGNI?

[Ellen Spertus]

It used to do something, which is why I’m only now getting the detekt warning.

[Mike]

Tough to answer without more context. But if the function serves a purpose, you can always override Detekt with an @Suppress. It doesn't know everything.

[Ellen Spertus]

Thanks for the reply, @Mike. I know I can suppress it but didn’t want to do that casually in case I misunderstood the reason for the rule.

[Mike]

Under normal circumstances, an empty function means you forgot to delete something, as it serves no obvious purpose. If you know it will serve a purpose, or you have to implement it because you're extending an abstract class that doesn't define a reasonable default, then you could end up with a legitimate empty function, and then Suppress would make sense.

Don't know if that helps explain my thought process better.

[Brais Gabin]

I don’t know if I understand you properly. You are using an API that forces you implement a function and you want to do nothing there? You should active ignoreOverriddenFunctions property. IMO this should be active by default.

[Brais Gabin]

If you are creating that api and your “default implementation” is do nothing you should add a comment inside the function to note. Something like: // no-op.

[Brais Gabin]

So I think that the rule is ok. If you really want to do nothing a comment tells others that this is not a bug. And if you are just fighting agains a bad API you can enable ignoreOverriddenFunctions