Executing a list of tasks from a database

Question

I have a list of tasks I get from my database (as strings) and then execute. However I'm uncertain on how to implement this correctly.

Currently I have implemented this in the following way:

Private Sub Monitoring(installationCode As String, drive As String, syncFolder As String)
 InitializeDatabaseConnection(installationCode)
 TraceInitialize(drive, "Monitoring")
 Trace.TraceInformation("Monitoring started for installation: " & installationCode)
 For Each param In JdnParamDAO.GetJdnParams(General.AmosRemoteTask)
 If param.Value Then
 RemoteTask.Deptid = Department.GetDepartmentsNotInOne().Where(Function(x) x.Code = "001").FirstOrDefault.DepartmentID
 Dim monitorTasks = TaskDefinitionDAO.GetActiveTaskDefinitionsByCategory(TaskDefinitionCategoryEnum.Monitor)
 Trace.TraceInformation(monitorTasks.Count() & " have been found.")
 'Perform each monitor task and save the results to the database
 For Each monitorTask In monitorTasks
 Try
 Trace.TraceInformation("MonitorTask: " & monitorTask.Description.ToLowerInvariant() & " found.")
 Dim result = String.Empty
 Select Case monitorTask.Description.ToLowerInvariant()
 Case "getlastbackup"
 result = RemoteTaskHelper.GetLastBackup(drive, installationCode)
 Case "getamosrtversion"
 result = General.AmosRTVersion
 Case "getstockondummypercentage"
 result = RemoteTaskHelper.GetStockOnDummyPercentage()
 Case "getnumberofhistoryrecords"
 result = RemoteTaskHelper.GetNumerOfHistoryRecords()
 Case "getnumberofsparepartlogrecords"
 result = RemoteTaskHelper.GetNumberOfSparePartLogRecords()
 Case "getinconsistenciesbetweencompjobnextdueandlastdone"
 result = RemoteTaskHelper.GetInconsistenciesBetweenCompJobNextDueAndLastDone()
 Case "getdifferencesofcompjobnextdueandworkorderdue"
 result = RemoteTaskHelper.GetDifferencesOfCompJobNextDueAndWorkOrderDue()
 Case "getinconsistenciesbetweencompjobsprioandworkordersprio"
 result = RemoteTaskHelper.GetWorkOrderCompJobPriority()
 Case "getactivecompjobswithnextdue"
 result = RemoteTaskHelper.GetActiveCompJobsWithNextDue()
 Case "getactivecompjobswithoutnextdue"
 result = RemoteTaskHelper.GetActiveCompJobsWithoutNextDue()
 Case "getopenworkorders"
 result = RemoteTaskHelper.GetOpenWorkOrders()
 Case "getamosguiversion"
 result = RemoteTaskHelper.GetAmosGuiVersion(syncFolder)
 Case "checkaddinsactivation"
 result = RemoteTaskHelper.CheckAddinsActivation()
 Case "checkamosmobileactivation"
 result = RemoteTaskHelper.CheckAmosMobileActivation()
 Case "checkchangelogactivation"
 result = RemoteTaskHelper.CheckChangelogActivation()
 Case "retrievejdnmodules"
 result = RemoteTaskHelper.RetrieveJdnModules(installationCode, syncFolder)
 Case "retrieveamosparameters"
 result = RemoteTaskHelper.RetrieveAmosParameters(installationCode, syncFolder)
 Case "retrievestocksituation"
 result = RemoteTaskHelper.RetrieveStockSituation(installationCode, syncFolder)
 Case "executesql"
 result = RemoteTaskHelper.ExecuteSQL(drive, installationCode)
 Case "getfreespace"
 result = RemoteTaskHelper.GetFreeSpace(drive)
 Case Else
 Trace.TraceInformation("Non excisting monitoring task: " + monitorTask.Description)
 ExceptionMonitor.LogExceptionEvent(
 "Non excisting monitoring task: " + monitorTask.Description, ProblemSeverity.Error)
 End Select
 If Not String.IsNullOrEmpty(result) Then
 Dim taskLog = New RemoteTaskLog()
 taskLog.CreationDate = DateTime.Now()
 taskLog.TaskDefinition = monitorTask
 taskLog.SourceInstallation = Int32.Parse(installationCode)
 taskLog.DestinationInstallation = CLng(General.AmosRTHeadOffice)
 taskLog.AppVersion = General.AmosRTVersion
 taskLog.TaskResult = result.ToString()
 RemoteTaskLogDAO.Save(taskLog)
 Else
 Trace.TraceInformation("No result returned for monitoring task: " + monitorTask.Description)
 ExceptionMonitor.LogExceptionEvent(
 "No result returned for monitoring task: " + monitorTask.Description, ProblemSeverity.Error)
 End If
 Catch ex As Exception
 Trace.TraceInformation("Error " + ex.ToString())
 ExceptionMonitor.LogExceptionEvent(ex, ProblemSeverity.Error)
 End Try
 Next
 End If
 Next
 End Sub

I am certain there is a better way to do this. I did some reading and always end up with the Strategy pattern (like asked here) but I don't seem to understand how to implement it in this situation.

Am I going in the correct direction here or not at all?

Answer 1

Important: Always set Option Explicit On

From what's an option strict and explicit?

Option Strict "restricts implicit data type conversions to only widening conversions". See [here][2]. With this option enabled, you can't accidentally convert one data type to another that is less precise (e.g. from an Integer to a Byte). Again, an option that should be turned on by default.

General

• by using guard conditions you can save horizontal spacing which improves readability. A guard condition is a condition which guards the code from beeing executed if one ore more certain conditions are fullfilled. This is done by either returning at this point or by throwing an exception depending on which is suitable in this case.

• extract the processing of param to a separate method.

• extract the processing of monitorTask to a separate method.

• extract the processing of the result to a separate method.

• comments should describe why something is done. What is done should be described by the code itself.

• calling ToString() on a String is not necessary.

this would lead for your Monitoringmethod to look like

Private Sub Monitoring(installationCode As String, drive As String, syncFolder As String)
 InitializeDatabaseConnection(installationCode)
 TraceInitialize(drive, "Monitoring")
 Trace.TraceInformation("Monitoring started for installation: " & installationCode)
 For Each param In JdnParamDAO.GetJdnParams(General.AmosRemoteTask)
 If Not param.Value Then continue For
 ProcessParameter(param)
 Next
End Sub
Private Sub ProcessParameter(param As someType, installationCode As String, drive As String, syncFolder As String)
 RemoteTask.Deptid = Department.GetDepartmentsNotInOne().Where(Function(x) x.Code = "001").FirstOrDefault.DepartmentID
 Dim monitorTasks = TaskDefinitionDAO.GetActiveTaskDefinitionsByCategory(TaskDefinitionCategoryEnum.Monitor)
 Trace.TraceInformation(monitorTasks.Count() & " have been found.")
 For Each monitorTask In monitorTasks
 Try
 Dim result as String = ProcessMonitorTasks(monitorTask.Description, installationCode, drive, syncFolder)
 ProcessMonitorResult(result, monitorTask, installationCode)
 Catch ex As Exception
 Trace.TraceInformation("Error " + ex.ToString())
 ExceptionMonitor.LogExceptionEvent(ex, ProblemSeverity.Error)
 End Try
 Next
End Sub
Private Function ProcessMonitorTasks(monitorTaskDescription As String, installationCode As String, drive As String, syncFolder As String) As String
 Select Case monitorTaskDescription.ToLowerInvariant()
 Case "getlastbackup"
 return RemoteTaskHelper.GetLastBackup(drive, installationCode)
 Case "getamosrtversion"
 return General.AmosRTVersion
 Case "getstockondummypercentage"
 return RemoteTaskHelper.GetStockOnDummyPercentage()
 Case "getnumberofhistoryrecords"
 return RemoteTaskHelper.GetNumerOfHistoryRecords()
 Case "getnumberofsparepartlogrecords"
 return RemoteTaskHelper.GetNumberOfSparePartLogRecords()
 Case "getinconsistenciesbetweencompjobnextdueandlastdone"
 return RemoteTaskHelper.GetInconsistenciesBetweenCompJobNextDueAndLastDone()
 Case "getdifferencesofcompjobnextdueandworkorderdue"
 return RemoteTaskHelper.GetDifferencesOfCompJobNextDueAndWorkOrderDue()
 Case "getinconsistenciesbetweencompjobsprioandworkordersprio"
 return RemoteTaskHelper.GetWorkOrderCompJobPriority()
 Case "getactivecompjobswithnextdue"
 return RemoteTaskHelper.GetActiveCompJobsWithNextDue()
 Case "getactivecompjobswithoutnextdue"
 return RemoteTaskHelper.GetActiveCompJobsWithoutNextDue()
 Case "getopenworkorders"
 return RemoteTaskHelper.GetOpenWorkOrders()
 Case "getamosguiversion"
 return RemoteTaskHelper.GetAmosGuiVersion(syncFolder)
 Case "checkaddinsactivation"
 return RemoteTaskHelper.CheckAddinsActivation()
 Case "checkamosmobileactivation"
 return RemoteTaskHelper.CheckAmosMobileActivation()
 Case "checkchangelogactivation"
 return RemoteTaskHelper.CheckChangelogActivation()
 Case "retrievejdnmodules"
 return RemoteTaskHelper.RetrieveJdnModules(installationCode, syncFolder)
 Case "retrieveamosparameters"
 return RemoteTaskHelper.RetrieveAmosParameters(installationCode, syncFolder)
 Case "retrievestocksituation"
 return RemoteTaskHelper.RetrieveStockSituation(installationCode, syncFolder)
 Case "executesql"
 return RemoteTaskHelper.ExecuteSQL(drive, installationCode)
 Case "getfreespace"
 return RemoteTaskHelper.GetFreeSpace(drive)
 Case Else
 Trace.TraceInformation("Non excisting monitoring task: " + monitorTask.Description)
 ExceptionMonitor.LogExceptionEvent(
 "Non excisting monitoring task: " + monitorTaskDescription, ProblemSeverity.Error)
 End Select
 return String.Empty
End Function
Private Sub ProcessMonitorResult(result As String, monitorTask As someObject, installationCode As String)
 If String.IsNullOrEmpty(result) Then
 Trace.TraceInformation("No result returned for monitoring task: " + monitorTask.Description)
 ExceptionMonitor.LogExceptionEvent(
 "No result returned for monitoring task: " + monitorTask.Description, ProblemSeverity.Error)
 Exit Sub
 End If
 Dim taskLog As RemoteTaskLog = New RemoteTaskLog()
 taskLog.CreationDate = DateTime.Now()
 taskLog.TaskDefinition = monitorTask
 taskLog.SourceInstallation = Int32.Parse(installationCode)
 taskLog.DestinationInstallation = CLng(General.AmosRTHeadOffice)
 taskLog.AppVersion = General.AmosRTVersion
 taskLog.TaskResult = result
 RemoteTaskLogDAO.Save(taskLog)
End Sub

Update:

I really don't like typing strings in code. I thought about using an Enum but I'm not entirely sure how that would work, any ideas?

This just depends on how monitorTask can/should be changed. If you want to use enums, the monitorTask will need to provide this enum too. Right now you are using the Description property which is a string.

If you want to/have to stick to the "string version" you should at least delare constants and use them in the Select..Case and whereever you need them.

• Shelby115: Correct me if I'm wrong, but the reason for switching up the select-case (among other things) is because the returns are going to prevent it from checking all cases after the one that succeeds (as the original is lacking breaks), Yes? If that is the case, I figured it was worth mentioning in the off-case that the OP was not aware of this reasoning or failed to realize.
• Heslacher: In Vb.NET there is no slipping through the cases. If the desired case is found the Select will be left.
• Schoof: Thanks a lot! This has been a great help. What exactly are guard conditions? Google has lead me to AndAlso but I don't believe that's correct? What I'm still wondering is if there isn't a better way to do that big Select Case? I really don't like typing strings in code. I thought about using an Enum but I'm not entirely sure how that would work, any ideas? :) Thanks again!
• Heslacher: Updated answer.
• Mathieu Guindon: @Thomas you may have better luck googling for "guard clause" instead ;)