sonar-delphi icon indicating copy to clipboard operation
sonar-delphi copied to clipboard

Add new rule: Methods should not be empty

Open fabriciocolombo opened this issue 9 years ago • 3 comments

There are several reasons for a method not to have a method body:

  • It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
  • It is not yet, or never will be, supported. In this case an [ENotImplemented|ENotSupportedException] should be raised.
  • The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Exception An abstract class' may have empty methods, in order to provide default implementations for child classes.

fabriciocolombo avatar Oct 09 '15 02:10 fabriciocolombo

Isn't this already kind of covered by "Empty 'begin' statement" rule? I see a lot of those violations, because I quite heavily use Null Object Pattern and, naturally, have a lot of empty methods.

Teloah avatar Dec 17 '15 16:12 Teloah

In a way yes, but empty 'begin' statement is very generic. Methods should not be empty is more specific. We'll leave this on hold for now.

fabriciocolombo avatar Dec 18 '15 02:12 fabriciocolombo

On a past team I worked, we used to just write an "Exit;" statement on virtual procedures that weren't abstract. Just because on Delphi 5 if you override an abstract method and call inherited, the IDE raises an abstract error. Above Delphi 6 that error doesn't appear anymore. So I think that situation should be considered.

marcosrocha85 avatar Feb 08 '16 16:02 marcosrocha85