patternMinor
Performing actions based on command line arguments
Viewed 0 times
performingactionsargumentslinebasedcommand
Problem
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
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
Solution
Based on the naming guidelines input parameters should be named using
Calling
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
The
Instead of using this loop you could use the
Using the good old
this would be much more readable like
If you want to initialize some public properties when you are creating the object, you can use the "new" way of using
The
Which is then called like
and
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 SubThe
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 SubUsing 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 SubWhich is then called like
Private Shared Sub ReplicationExport(ByVal range As String)
Trace.TraceInformation("Creating ReplicationExport Task.")
ExecuteReplication(range, Business.ReplicationType.Export)
End Suband
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 SubCode Snippets
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 SubPrivate Shared Sub WriteArgs(ByVal cmdArgs() As String)
Trace.TraceInformation("Arguments: " & String.Join(" ", cmdArgs))
End SubDim task = New UpgradeSybaseDatabases
With task
.ScriptFolder = scriptFolder
End With
task.Start()Dim task = New UpgradeSybaseDatabases
task.ScriptFolder = scriptFolder
task.Start()Dim task = New UpgradeSybaseDatabases With { .ScriptFolder = scriptFolder }
tast.Start()Context
StackExchange Code Review Q#79575, answer score: 4
Revisions (0)
No revisions yet.