Use 'Select Case' selectively

Using 'Select Case' inappropriately can produce code that is a maintenance nightmare. This article proposes a more maintainable alternative.

NEW: Here is an adendum to the article.

One day I had to review my buddy's user interface code for controlling a significant part of each congress' operations - the parliamentary voting process. The UI code had to interact with the voting engine by means of the Voting class contract:

Public NotInheritable Class Voting
...
Public Delegate Sub VotingProgressEventHandler(...)

Public Event VotingProgress As VotingProgressEventHandler
Public Event VotingTimedOut As VotingProgressEventHandler
Public Event VotingStarted As VotingProgressEventHandler
Public Event VotingPaused As VotingProgressEventHandler
Public Event VotingResumed As VotingProgressEventHandler
Public Event VotingStopped As VotingProgressEventHandler

Public ReadOnly Property VotingInProgress() As Boolean
Public ReadOnly Property VotingSuspended() As Boolean

Public Sub StartVoting()
Public Sub StopVoting()
Public Sub SuspendVoting()
Public Sub ResumeVoting()

Public Function GetVotingResults() As VotingResults
...
End Class

Janko (that's my buddy's name) is pretty good at designing and implementing nice, easy to use user interfaces even for complex applications. He showed me his voting form and I was impressed (take a look at the screenshot - Slovak language only, 65KB).

When I looked at the source code, I've seen that Janko defined an Enum for the voting engine states:

Public Enum VotingState
  NotRunning
  InProgress
  Suspended
  Finished
End Enum

The difference between NotRunning and Finished states is that in the former case, voting results aren't available yet.

Public Class FVoting
  Inherits System.Windows.Forms.Form

  Private _VotingState As VotingState
  ...
  Private Sub EnableCmds()
    Select Case _VotingState
      Case VotingState.NotRunning
        EnableStartCmds(True)
        EnableStopCmds(False)
        EnableSuspendCmds(False)
        EnablePrintCmds(False)
      Case VotingState.InProgress
        EnableStartCmds(False)
        EnableStopCmds(True)
        EnableSuspendCmds(True)
        EnablePrintCmds(False)
      Case VotingState.Suspended
        ...
      Case VotingState.Finished
        ...
    End Select
  End Sub

  Private Sub RefreshStatus()
    Dim sMSG As String
    Select Case _VotingState
      Case VotingState.NotRunning
        sMSG = String.Format("...")
      Case VotingState.InProgress
        sMSG = String.Format("...", Me.Number.Text)
      Case VotingState.Suspended
        sMSG = String.Format("...", Me.Number.Text)
      Case VotingState.Finished
        sMSG = String.Format("...", Me.Number.Text)
    End Select
    StatusText(sMSG)
  End Sub
...
End Class

First, the Select Case statements don't include the Case Else clause. I've been always recommending "catching" those "impossible" cases as soon as possible, for example:

Private Sub RefreshStatus()
    Dim sMSG As String
    Select Case _VotingState
      Case VotingState.NotRunning
        sMSG = String.Format("...")
      ...
      Case Else
        Debug.Assert(False, "Unexpected state: " & _
                     _VotingState.ToString())
    End Select
    StatusText(sMSG)
End Sub

Second, and most important, the code is difficult to maintain and enhance.

What do I mean?

Well, imagine that a new voting state is introduced in the future. Each and every place where the Select Case statement is used has to be revised, and another Case branch has to be added and debugged.

And what if the poor maintenance programmer forgets to update one of the places? Depending on the usage pattern, that kind of bug might bite long after the application is in production.

So what would be a better solution?

The VotingState enum defines...well, states. So why not to use a variation of the State Machine design pattern?

Let's see how it can be done:

First, I'd get rid of the VotingState enum. Instead, I'd define an abstract base class - VotingStateBase:

Public Class FVoting
  Inherits System.Windows.Forms.Form
  ...
  Public MustInherit Class VotingStateBase
    Private _Owner As FVoting
    Friend Sub New(ByVal owner As FVoting)
      _Owner = owner
      Debug.Assert(Not _Owner Is Nothing)
    End Sub
    Public MustOverride Sub RefreshStatus()
    Public MustOverride Sub EnableCmds()
  End Class
...
End Class

The actual VotingStateBase descendants would represent the actual voting states and associated operations, for example:

  Public Class NotRunningVotingState
    Inherits VotingStateBase
    Public Sub New(ByVal owner As FVoting)
      MyBase.New(owner)
    End Sub
    Public Overrides Sub RefreshStatus()
      _Owner.StatusText("...")
    End Sub
    Public Overrides Sub EnableCmds()
      _Owner.EnableStartCmds(True)
      _Owner.EnableStopCmds(False)
      _Owner.EnableSuspendCmds(False)
      _Owner.EnablePrintCmds(False)
    End Sub
  End Class

Public Class FVoting
  Inherits System.Windows.Forms.Form
  Private _VotingState As VotingStateBase
  ...
  Private Sub EnableCmds()
    _VotingState.EnableCmds()
  End Sub
  Private Sub RefreshStatus()
    _VotingState.RefreshStatus()
  End Sub
...
End Class

Verbose?

Well, it might be. But it pays for itself at the moment you'll need to add an additional state, or to modify the behavior for an existing state:

You have to do it at one place only!

And...you can't forget to handle any of the operations previously straggled throughout the source code, because they are now defined as MustInherit methods of the base class.

Watch out when you find yourself defining an Enum and coding Select Case branching according to the Enum's value. There might be a more maintainable solution, such as the one just described.

© Palo Mraz, Sunday, August 17, 2003

Links