Context: The following code is used to look up a setting values based on multiple layers of configuration within the application. For any given setting (ie: default printer), the application can be configured at the following levels in order of precedence.
- Operation
- Material
- Workstation & Program Combination
- Workstation
- Program
- User
- Plant
When the application requires a setting, it performs a lookup by providing as much information as it has available in context then searches each level and uses the first value it finds. If no value is found, the caller will handle the default action.
Logic Layer
This is the function that will be called by any part of the application that needs to look up a setting value. The caller must specify the setting ID and one or more of the other parameters. The purpose here is to allow the caller to look up just "Workstation" level settings for example.
Public Function SettingLookup(operationID As String,
materialID As String,
userID As String,
plantID As String,
workstationID As String,
programID As String,
settingID As String
) As String()
If settingID = "" Then
Throw New Exception("Setting ID Required")
End If
Dim settingValue = SettingLookupHelper(operationID, materialID, userID, plantID, workstationID, programID, settingID)
If settingValue IsNot Nothing Then
Return JsonConvert.DeserializeObject(Of String())(settingValue)
Else
Return Nothing
End If
End Function
This function is used to access the data access layer and get the settings from the database.
Private Function SettingLookupHelper(operationID As String,
materialID As String,
userID As String,
plantID As String,
workstationID As String,
programID As String,
settingID As String
) As String
Dim settingValue As Setting
If operationID <> "" Then
settingValue = GetOperationSetting(operationID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
If materialID <> "" Then
settingValue = GetMaterialSetting(materialID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
If workstationID <> "" And programID <> "" Then
settingValue = GetWorkstationProgramSetting(workstationID, programID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
If workstationID <> "" Then
settingValue = GetWorkstationSetting(workstationID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
If programID <> "" Then
settingValue = GetProgramSetting(programID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
If userID <> "" Then
settingValue = GetUserSetting(userID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
If plantID <> "" Then
settingValue = GetPlantSetting(plantID, settingID)
If settingValue IsNot Nothing Then
Return settingValue.Value
End If
End If
Return Nothing
End Function
Data Access Layer
This is an example of the code I have in the data access layer for each of the configuration levels listed above. Each of these files contains code to access a Settings table where the settings are defined and a many-to-many join table (ie: Material_Settings or Operation_Settings) that is used to set the setting values and link it to the entity like a material, operation, or user. There is a lot of nearly duplicated code in my data access layer to support this design.
Public Class MaterialSetting
Inherits Setting
Private _MaterialID As String
Public Property Material_ID() As String
Get
Return _MaterialID
End Get
Set(ByVal value As String)
_MaterialID = value
End Set
End Property
Private _Material As String
Public Property Material() As String
Get
Return _Material
End Get
Set(ByVal value As String)
_Material = value
End Set
End Property
End Class
Public Module MaterialSettingsUtilities
Public Function GetMaterialSetting(materialID As String, settingID As String) As MaterialSetting
Dim parameters As New Dictionary(Of String, Object)
parameters.Add("Material_ID", materialID)
parameters.Add("Setting_ID", settingID)
Dim queryCondition As String
queryCondition = " WHERE Material_ID = HEXTORAW(:Material_ID) AND Setting_ID = HEXTORAW(:Setting_ID) "
Dim settings As Settings = GetMaterialSettingsFiltered(parameters, queryCondition)
If settings.Count > 0 Then
Return CType(settings(0), MaterialSetting)
Else
Return Nothing
End If
End Function
Private Function GetMaterialSettingsFiltered(parameters As Dictionary(Of String, Object), queryCondition As String) As Settings
Dim query As String
query = " SELECT * "
query += " FROM (Materials NATURAL JOIN Material_Settings) "
query += " NATURAL JOIN Settings "
query += queryCondition
Using conn As New OracleConnection(GetConnectionString("WeighScaleDB"))
Using cmd = CreateOracleCommand(query, parameters, conn)
conn.Open()
Using dr = cmd.ExecuteReader
Dim settings As New Settings
If Not dr.Read() Then
Return settings
End If
Do
settings.Add(New MaterialSetting With {
.Setting_ID = ConvertByteArrayToString(TryCast(dr("Setting_ID"), Byte())),
.Material_ID = ConvertByteArrayToString(TryCast(dr("Material_ID"), Byte())),
.Material = dr("Material").ToString(),
.Value = dr("Setting_Value").ToString(),
.Name = dr("Setting_Name").ToString(),
.Description = dr("Setting_Description").ToString(),
.DataType = dr("Setting_Data_Type").ToString(),
.Category = dr("Setting_Category").ToString()
})
Loop While dr.Read()
Return settings
End Using
End Using
End Using
End Function
End Module
This is the Settings object from which each of the subtypes (ie: MaterialSetting) are derived.
Public Class Setting
Private _SettingID As String
Public Property Setting_ID() As String
Get
Return _SettingID
End Get
Set(ByVal value As String)
_SettingID = value
End Set
End Property
Private _SettingName As String
Public Property Name() As String
Get
Return _SettingName
End Get
Set(ByVal value As String)
_SettingName = value
End Set
End Property
Private _SettingDescription As String
Public Property Description() As String
Get
Return _SettingDescription
End Get
Set(ByVal value As String)
_SettingDescription = value
End Set
End Property
Private _SettingDataType As String
Public Property DataType() As String
Get
Return _SettingDataType
End Get
Set(ByVal value As String)
_SettingDataType = value
End Set
End Property
Private _SettingCategory As String
Public Property Category() As String
Get
Return _SettingCategory
End Get
Set(ByVal value As String)
_SettingCategory = value
End Set
End Property
Private _SettingValue As String
Public Property Value() As String
Get
Return _SettingValue
End Get
Set(ByVal value As String)
_SettingValue = value
End Set
End Property
End Class
Any constructive thoughts on would be appreciated. I can post examples of my table structures or any of the other helper or DAL functions shown in this code.
-
\$\begingroup\$ May I ask why you're not using Encoding.GetString? \$\endgroup\$Bjørn-Roger Kringsjå– Bjørn-Roger Kringsjå2015年11月05日 18:13:10 +00:00Commented Nov 5, 2015 at 18:13
-
\$\begingroup\$ @Bjørn-RogerKringsjå Thanks for the tip. I didn't know about that function. \$\endgroup\$Kent Anderson– Kent Anderson2015年11月05日 19:13:25 +00:00Commented Nov 5, 2015 at 19:13
1 Answer 1
SettingLookup
If settingID = "" Then Throw New Exception("Setting ID Required") End If
You shouldn't just throw an
Exception
but you should throw anArgumentException
. From the design guidelines for exceptionDo throw the most specific (the most derived) exception that is appropriate. For example, if a method receives a null (Nothing in Visual Basic) argument, it should throw System.ArgumentNullException instead of its base type System.ArgumentException.
IMO you should test if the
settingID
isnull
(Nothing) too.If settingID Is Nothing Then Throw New ArgumentNullException("settingID") Else If settingID = String.Empty Then Throw New ArgumentException("Setting ID Required") End If
here
If settingValue IsNot Nothing Then Return JsonConvert.DeserializeObject(Of String())(settingValue) Else Return Nothing End If
the
Else
is redundant because if theIf
condition istrue
theElse
part is nevver hit.If settingValue IsNot Nothing Then Return JsonConvert.DeserializeObject(Of String())(settingValue) End If Return Nothing
SettingLookupHelper
SettingLookupHelper
is a noun but method names should be made out of a verb or a verb phrase. Please see: NET naming guidelineinstead of checking against
""
I would like to suggest to check againstString.Empty
which is IMO more readable at least if you are getting older and your eyes are getting bad you will agree.
MaterialSetting & Setting
If you are using VS 2010 you can/should use the auto-implemented properties at least if you aren't doing any validation in the setter and you don't need different acessobility modifiers (Private
, Public
etc.)
Your MaterialSetting
would then become this
Public Class MaterialSetting
Inherits Setting
Public Property Material_ID As String
Public Property Material As String
End Class
and the Setting
class will look like so
Public Class Setting
Public Property Setting_ID As String
Public Property Name As String
Public Property Description As String
Public Property DataType As String
Public Property Category As String
Public Property Value As String
End Class
looks much cleaner, doesn't it ?
GetMaterialSetting
Looks good expect for the redundant Else
.
GetMaterialSettingsFiltered
The String concatenation
Dim query As String query = " SELECT * " query += " FROM (Materials NATURAL JOIN Material_Settings) " query += " NATURAL JOIN Settings " query += queryCondition
will create each time a new String
object because strings are immutable. You should consider to use the line continuation _
like so
query = " SELECT * " _
+ " FROM (Materials NATURAL JOIN Material_Settings) " _
+ " NATURAL JOIN Settings " _
+ queryCondition
You should think about wether it would be better to only include the fields you are needing into the Select
instead of returning all columns. Sure you can say that you need all of them, but what will happen if you or someone else add more columns to that datatable which are needed for something else, you will still return all of them.
You are using Using
that's good.
You are using a parameterized query that's good too.
You should consider to extract the actual data retrieval to a separate data access layer which shouldnÄ't be aware of any of these MaterialSetting
's (or any other classes) which just returns either a DataSet
or a DataTable
.
In this way you could use it everywhere in your projects.
Something along these lines
Public Function GetRecords(parameters As Dictionary(Of String, Object), query As String, connectionString As String) As DataTable
Using DataTable table As New DataTable()
Using conn As New OracleConnection(connectionString)
Using cmd = CreateOracleCommand(query, parameters, conn)
Using adapter As New OracleDataAdapter(cmd)
adapter.Fill(table)
Return table
End Using
End Using
End Using
End Using
End Function
which could then be called like so
Private Function GetMaterialSettingsFiltered(parameters As Dictionary(Of String, Object), queryCondition As String) As Settings
Dim query As String = " SELECT * " _
+ " FROM (Materials NATURAL JOIN Material_Settings) " _
+ " NATURAL JOIN Settings " _
+ queryCondition
Dim settings As New Settings()
Dim table As DataTable = GetRecords(parameters, query, GetConnectionString("WeighScaleDB"))
If table Is Nothing Then
Return settings
End If
For Each row As DataRow In table.Rows
settings.Add(New MaterialSetting With {
.Setting_ID = ConvertByteArrayToString(TryCast(row("Setting_ID"), Byte())),
.Material_ID = ConvertByteArrayToString(TryCast(row("Material_ID"), Byte())),
.Material = row("Material").ToString(),
.Value = row("Setting_Value").ToString(),
.Name = row("Setting_Name").ToString(),
.Description = row("Setting_Description").ToString(),
.DataType = row("Setting_Data_Type").ToString(),
.Category = row("Setting_Category").ToString()
})
Next
Return settings
End Function
The name GetRecords()
isn't choosen that well maybe you can come up with a better one.