Procedure has an empty body
ID |
vbnet.maintainability.empty_function_body |
Severity |
high |
Remediation Complexity |
medium |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Dead Code |
Language |
VB.NET |
Description
Reports a Sub or Function declaration whose body contains no statements at all, including empty event handlers and empty implementations of an interface member. Declarations that are bodiless by design are not reported: constructors, Partial method declarations whose body is supplied in another part of the type, <DllImport> P/Invoke signatures, and the designer-owned InitializeComponent, as well as interface members and MustOverride declarations, which have no body to leave empty.
Rationale
An empty body gives a reader no way to tell an unfinished stub from a deliberate decision to do nothing, and both possibilities are wrong in a different way. An empty Sub accepts the call, does not perform the work and reports success, so the caller has nothing to notice and the missing effect surfaces as absent output somewhere else entirely. An empty Function is worse: Visual Basic returns the type’s default value, Nothing for a reference type, zero for a number, False for a Boolean, so the caller receives a plausible value and the defect typically appears far from here as a null reference or a silently skipped branch. An empty handler is the same trap applied to an event that looks wired up.
The following code illustrates the pattern detected by this rule:
Protected MustOverride Sub Paint(ByVal widget As Object)
' BAD: the interface method was stubbed out and never implemented.
' FLAGGED: Procedure has an empty body
Public Sub Render(ByVal widget As Object) Implements IWidgetRenderer.Render
End SubRemediation
Most findings are empty event handlers, and for those there is only one fix: delete the handler and the wiring that reaches it. A handler created by double-clicking a control in the designer, or left behind after its body was moved elsewhere, has no callers to chase and nothing to implement - remove the Sub together with its Handles clause, or the AddHandler line, or the OnClick attribute in the markup that names it. Do not fill it with a comment or a NotImplementedException: an event that fires and throws is worse than one that was never subscribed. For an ordinary procedure, implement it, or delete it together with the calls that reach it when the work turns out not to be needed - an unused empty procedure is dead code that reads as a feature. When doing nothing really is the correct behaviour, state that in code so it survives review and can be searched for: log the skipped operation, as in Debug.WriteLine("nothing buffered; flush is a no-op for this renderer"), or return an explicit value such as Return String.Empty rather than falling off the end of the Function. While the implementation of a genuine procedure is still outstanding, Throw New NotImplementedException() makes the gap fail loudly at the point of use instead of hiding it.