Performing actions based on command line arguments

Question

I'm currently reviewing a bit of an older console application which performs certain tasks depending on the command line argument that is given. These tasks are called through Windows Task Scheduler.

However reflecting on this I find the code to be ugly and I'm wondering if there is a better way to do something like this.

Public Shared Sub ParseTasks(ByVal CmdArgs() As String)
 ConvertAndWriteArgs(CmdArgs)
 Try
 Select Case CmdArgs(0)
 Case "SCRIPTS"
 'SCRIPTS has 1 input parameter: ScheduleType
 If CmdArgs.Length < 2 Then
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 Else
 Scripts(sheduleType:=CmdArgs(1).ToString)
 End If
 Case "CHECKTRANSFERREPLICATION"
 CheckTransferReplication()
 Case "CREATESYBASEBACKUP11"
 CreateSybaseBackup11()
 Case "SQLCOMMAND"
 SqlCommand()
 Case "REPLICATION_IN"
 'Replication In has 1 input parameter: Range
 If CmdArgs.Length < 2 Then
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 Else
 ReplicationImport(range:=CmdArgs(1).ToString())
 End If
 Case "REPLICATION_OUT"
 'Replication Out has 1 input parameter: Range
 If CmdArgs.Length < 2 Then
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 Else
 ReplicationExport(Range:=CmdArgs(1).ToString)
 End If
 Case "REEXPORTFILE"
 'Re export file Out has 1 input parameter: FileName
 If CmdArgs.Length < 2 Then
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 Else
 ReExportFile(fileName:=CmdArgs(1).ToString)
 End If
 Case "UPGRADE"
 'Upgrade has 1 input parameter: ScriptFolder
 If CmdArgs.Length < 2 Then
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 Else
 Upgrade(scriptFolder:=CmdArgs(1).ToString)
 End If
 Case "DUPLICATES"
 Duplicates()
 Case Else
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 End Select
 Catch ex As Exception
 ExceptionLogger.LogExceptionEvent(ex, Severity.Error)
 End Try
 End Sub
#Region "Task functions"
 Private Shared Sub AutoScripts(ByVal sheduleType As String)
 Trace.TraceInformation("Creating AutoScripts Task.")
 Dim task = New AutoScripts
 With task
 .ScheduleType = ConvertScheduleType(sheduleType)
 End With
 Trace.TraceInformation("Schedule Type: " & task.ScheduleType.ToString)
 task.Start()
 End Sub
 Private Shared Sub CheckTransferReplication()
 Trace.TraceInformation("Creating CheckTransferReplication Task.")
 Dim task = New CheckTransferReplication
 task.Start()
 End Sub
 Private Shared Sub CreateSybaseBackup11()
 Trace.TraceInformation("Creating CreateSybaseBackup11 Task.")
 Dim task = New CreateSybaseBackup
 task.Start()
 End Sub
 Private Shared Sub SqlCommand()
 Trace.TraceInformation("Creating SqlCommand Task.")
 Dim task = New Business.Task.ExecuteSqlCommand
 task.Start()
 End Sub
 Private Shared Sub ReExportFile(ByVal fileName As String)
 Trace.TraceInformation("Creating ReExportFile Task.")
 Dim task = New ReExportFile
 With task
 .FileName = fileName
 End With
 task.Start()
 End Sub
 Private Shared Sub Upgrade(ByVal scriptFolder As String)
 Trace.TraceInformation("Creating UpgradeSybaseDatabases Task.")
 Dim task = New UpgradeSybaseDatabases
 With task
 .ScriptFolder = scriptFolder
 End With
 task.Start()
 End Sub
#Region "Replication"
 Private Shared Sub ReplicationImport(ByVal range As String)
 Trace.TraceInformation("Creating ReplicationImport Task.")
 Dim task = New ExecuteReplication
 task.ReplicationType = Business.ReplicationType.Import
 If range.Equals("001", StringComparison.OrdinalIgnoreCase) Then
 task.ReplicationGroup = Business.ReplicationGroup.HeadOffice
 task.Start()
 Try
 EdimarObjectDAO.ProcessRefErrors()
 Catch ex As Exception
 ExceptionLogger.LogExceptionEvent(ex, ProblemSeverity.Error)
 End Try
 Dim sqlCommandTask = New ExecuteSqlCommand
 SqlCommandTask.Start()
 Else
 task.ReplicationGroup = Business.ReplicationGroup.Sattelite
 task.ReplicationSatteliteRange = range
 task.Start()
 End If
 End Sub
 Private Shared Sub ReplicationExport(ByVal range As String)
 Trace.TraceInformation("Creating ReplicationExport Task.")
 Dim task = New ExecuteReplication
 task.ReplicationType = Business.ReplicationType.Export
 If Range.Equals("001", StringComparison.OrdinalIgnoreCase) Then
 task.ReplicationGroup = Business.ReplicationGroup.HeadOffice
 task.Start()
 Else
 task.ReplicationGroup = Business.ReplicationGroup.Sattelite
 task.ReplicationSatteliteRange = Range
 task.Start()
 End If
 End Sub
 Private Shared Sub ProcessLogFiles()
 Trace.TraceInformation("Processing Replication Logfiles.")
 Dim task = New ProcessLogFiles
 task.Start()
 End Sub
#End Region
#End Region
#Region "Helper Functions"
 Private Shared Sub ConvertAndWriteArgs(ByVal CmdArgs() As String)
 Dim strArgs = String.Empty
 For Each arg In CmdArgs
 If strArgs = String.Empty Then
 strArgs = arg
 Else
 strArgs += " " & arg
 End If
 Next
 Trace.TraceInformation("Arguments: " & strArgs)
 End Sub
 Private Shared Function ConvertScheduleType(ByVal scheduleType As String) As Business.ScheduleType
 Dim schedule As Business.ScheduleType
 Select Case ScheduleType
 Case "M"
 schedule = Business.ScheduleType.Monthly
 Case "W"
 schedule = Business.ScheduleType.Weekly
 Case "W2"
 schedule = Business.ScheduleType.Weekly2
 Case "D"
 schedule = Business.ScheduleType.Daily
 Case "D2"
 schedule = Business.ScheduleType.Daily2
 Case Else
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", ProblemSeverity.Error)
 End Select
 Return schedule
 End Function
#End Region

Answer 1

I don't see any issues with your switch itself. There will only ever be a finite number of arguments to be handled and this seems like a reasonable way to go about it. What I don't like is how you've repeated the check on CmdArgs.Length all over the code. (I do like your comments though. They've really clarified unclear things.)

I would recommend extracting the logic into a routine that takes a delegate function. You could declare it something like this.

Delegate Sub ExecuteRoutine(range As String)

And gets used in another (poorly named) method.

Private Sub CheckArgCountAndExecute(cmdArgs() As String, toExecute As ExecuteRoutine)
 If cmdArgs.Length < 2 Then
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 Else
 toExecute(range:=CmdArgs(1).ToString())
 End If
End Sub

And gets called in your switch like this.

Case "REPLICATION_IN"
 'Replication In has 1 input parameter: Range
 CheckArgCountAndExecute(CmdArgs(1), AddressOf ReplicationImport)
Case "REPLICATION_OUT"
 'Replication Out has 1 input parameter: Range
 CheckArgCountAndExecute(CmdArgs(1), AddressOf ReplicationExport)

You can read more about delegate functions here.

• Delegates (Visual Basic)
• How to: Invoke a Delegate Method (Visual Basic)
• How to: Pass Procedures to Another Procedure in Visual Basic

---

I agree with @Heslacher about this particular With statement, but I think it's a bit foolish to say that all Withs are bad. It's just senseless to use it for what could be one line of code.

 With task
 .ScheduleType = ConvertScheduleType(sheduleType)
 End With

That is much better as

task.ScheduleType = ConvertScheduleType(scheduleType)

However, I would say it would be valid to switch over to one here.

Private Shared Sub ReExportFile(ByVal fileName As String)
 Trace.TraceInformation("Creating ReExportFile Task.")
 Dim task = New ReExportFile
 With task
 .FileName = fileName
 End With
 task.Start()
End Sub

VS

Private Shared Sub ReExportFile(ByVal fileName As String)
 Trace.TraceInformation("Creating ReExportFile Task.")
 Dim task = New ReExportFile
 With task
 .FileName = fileName
 .Start()
 End With
End Sub

• Schoof: Thanks a lot! The delegate functions are really quite useful. :) I like both the answers on my question, but I can only choose one, this is difficult!
• RubberDuck: You're welcome! I find them a handy screwdriver for the toolbox too! Glad I could help.

Answer 2

Based on the naming guidelines input parameters should be named using camelCase casing.

---

Calling ToString() on a String doesn't really add any value. A String is a String is a String.

---

You have a lot of duplicated code here which can be reduced by extracting the cases of single argument calls to a separate method like

Public Shared Sub ParseTasks(ByVal cmdArgs() As String)
 ConvertAndWriteArgs(cmdArgs)
 if CmdArgs.Length = 1 Then
 ParseTasks(cmdArgs(0))
 Exit Sub
 End if
 Dim argument as String = cmdArgs(1)
 Try
 Select Case cmdArgs(0)
 Case "SCRIPTS"
 Scripts(sheduleType:=argument)
 Case "REPLICATION_IN"
 ReplicationImport(range:=argument)
 Case "REPLICATION_OUT"
 ReplicationExport(Range:=argument)
 Case "REEXPORTFILE"
 ReExportFile(fileName:=argument)
 Case "UPGRADE"
 Upgrade(scriptFolder:=argument)
 Case Else
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 End Select
 Catch ex As Exception
 ExceptionLogger.LogExceptionEvent(ex, Severity.Error)
 End Try
End Sub
Private Shared Sub ParseTasks(ByVal argument As String)
 Try
 Select Case argument 
 Case "CHECKTRANSFERREPLICATION"
 CheckTransferReplication()
 Case "CREATESYBASEBACKUP11"
 CreateSybaseBackup11()
 Case "SQLCOMMAND"
 SqlCommand()
 Case "DUPLICATES"
 Duplicates()
 Case Else
 ExceptionLogger.LogExceptionEvent("Invalid Arguments", Severity.Error)
 End Select
 Catch ex As Exception
 ExceptionLogger.LogExceptionEvent(ex, Severity.Error)
 End Try
End Sub

---

The ConvertAndWriteArgs() method doesn't do any converting. It just adding spaces between the arguments and then writing the result to trace.

Instead of using this loop you could use the String.Join() method.

Private Shared Sub WriteArgs(ByVal cmdArgs() As String)
 Trace.TraceInformation("Arguments: " & String.Join(" ", cmdArgs))
End Sub 

---

Using the good old with shouldn't be done nowadays. If you have the feeling you should use it, don't do it if it isn't needed like this

Dim task = New UpgradeSybaseDatabases
With task
 .ScriptFolder = scriptFolder
End With
task.Start()

this would be much more readable like

Dim task = New UpgradeSybaseDatabases
task.ScriptFolder = scriptFolder
task.Start() 

If you want to initialize some public properties when you are creating the object, you can use the "new" way of using With like

Dim task = New UpgradeSybaseDatabases With { .ScriptFolder = scriptFolder }
tast.Start()

---

The ReplicationImport() and ReplicationExport() methods share a lot of duplicate code. You better extract this duplication to a separate method which is called by these methods.

Private Shared Sub ExecuteReplication(ByVal range As String, ByVal replicationType As Business.ReplicationType) 
 Dim task = New ExecuteReplication
 task.ReplicationType = replicationType 
 If range.Equals("001", StringComparison.OrdinalIgnoreCase) Then
 task.ReplicationGroup = Business.ReplicationGroup.HeadOffice
 Else
 task.ReplicationGroup = Business.ReplicationGroup.Sattelite
 task.ReplicationSatteliteRange = range
 End If
 task.Start()
End Sub 

Which is then called like

Private Shared Sub ReplicationExport(ByVal range As String)
 Trace.TraceInformation("Creating ReplicationExport Task.")
 ExecuteReplication(range, Business.ReplicationType.Export)
End Sub

and

Private Shared Sub ReplicationImport(ByVal range As String)
 Trace.TraceInformation("Creating ReplicationImport Task.")
 ExecuteReplication(range, Business.ReplicationType.Import)
 If range.Equals("001", StringComparison.OrdinalIgnoreCase) Then
 Try
 EdimarObjectDAO.ProcessRefErrors()
 Catch ex As Exception
 ExceptionLogger.LogExceptionEvent(ex, ProblemSeverity.Error)
 End Try
 Dim sqlCommandTask = New ExecuteSqlCommand
 SqlCommandTask.Start()
 End If
End Sub

• Schoof: Thanks a lot! That is already a great help. I wish I could see all these things, are there any books you can recommend that help with this or does it mostly come with experience?
• Heslacher: A good book is Clean Code by Robert C.Martin. But most is coming from doing ;-) and also beeing active here on code review.
• RubberDuck: There's enough good stuff here that you got my upvote, but I'm not a fan of how the switch is spread across two different methods.