HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Encapsulation with a dbParameter class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withdbparameterencapsulationclass

Problem

I am using the DatabaseHelper class to execute SQL statements e.g. ExecuteDataReader, ExecuteDataTable etc. The signature for ExecuteReader is:

Public Overloads Shared Function ExecuteReader(ByVal connectionString As String, _
ByVal commandType As CommandType, _
ByVal commandText As String, _
ByVal ParamArray commandParameters() As DbParameter) As DbDataReader


Therefore it is expecing some parameters. I have created the class below:

`Public Class clsParameterValues
'Implements IDisposable

Private paramValues(0) As DbParameter

Public Function AssignParameterValues(ByVal strParameterName As String, ByVal strParameterValue As String, ByVal intDatabaseType As Integer) As Integer
Dim intArrayBound As Integer

intArrayBound = UBound(paramValues)
'If intArrayBound > 0 Then
If paramValues(0) Is Nothing = False Then
intArrayBound = intArrayBound + 1
ReDim Preserve paramValues(intArrayBound)
End If

If intDatabaseType = 1 Then

paramValues(intArrayBound) = New SqlParameter(strParameterName, strParameterValue)
ElseIf intDatabaseType = 2 Then
paramValues(intArrayBound) = New OracleParameter(strParameterName, strParameterValue)
'paramValues(intArrayBound) = New OracleParameter(":" & strParameterName, OracleType.Int32)
'paramValues(intArrayBound).Value = strParameterValue
End If
Return intArrayBound
End Function

Public Function AssignParameterValues(ByVal strParameterName As String, ByVal strParameterValue As Date, ByVal intDatabaseType As Integer) As Integer
Dim intArrayBound As Integer

intArrayBound = UBound(paramValues)
'If intArrayBound > 0 Then
If paramValues(0) Is Nothing = False Then
intArrayBound = intArrayBound + 1
ReDim P

Solution

Potential issues

-
intDatabaseType should be promoted to an instance variable instead of being supplied in every call to AssignParameterValues - the way you have it, you can build an 5-items array with 2 parameters for SQL Server and 3 parameters for Oracle. Not sure that's the way it was intended...

-
Your "database type" is a magic number. What tells client code to pass a 1 for a SQL Server database and a 2 for an Oracle database? Nothing. Not even an exception. That parameter is literally begging to be an enum type.

-
If ExecuteReader is ultimately taking in the return value from getParameterValues, then there's no need for ParamArray here; consider changing it to take any IEnumerable(Of DbParameter).

-
AssignParameterValues will resize the array even if intDataBaseType is an invalid value, so the return value is meaningless. Why are you returning the array's upper bound? Why not make it a Sub that can either succeed or throw an exception?

-
Instead of an array of DbParameter, consider using a List(Of DbParameter). That alone would make all of that ReDim Preserve and UBound stuff go away!

Naming issues

  • Hungarian notation is outdated, ugly at best and misleading at worst. Do yourself a favor and rename those variables!



  • clsParameterValues => ParameterValues



  • intArrayBound => arrayUpperBound



  • intDatabaseType => databaseType



  • strParameterName => name



  • strParameterValue => value



Other nitpicks

-
intArrayBound = intArrayBound + 1 can be shortened to intArrayBound += 1.

-
Prefer If Not SomeBooleanExpression Then for negation and If SomeBooleanExpression Then for the opposite, instead of the redundant If SomeBooleanExpression = True Then.

Context

StackExchange Code Review Q#31502, answer score: 2

Revisions (0)

No revisions yet.