Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Naming
On the whole, your naming is just fine. I wonder, though, why you called your class GenericDataModel? What makes it generic? Can't it be called DataModel?

Comments
Since your method naming and parameter naming are descriptive - most of your comments are completely redundant - I don't need a comment to tell me that CallSprocsInTransaction call sprocs in transaction, or that sprocsToCall is a list of sprocs to call... These comments just add clutter and nothing else.
This also goes for inline comments, which tell us trivial things like you are creating the command you are going to send.

Potential leak - not using using
When you create the connection inside the method, you Close it at the end of the method, but you do not count for possible Exceptions inside the method. In such a case - the connection will not be closed.
To prevent it you can use try...finally, but you can also take advantage of the fact that using supports null objects using supports null objects:

public void ExecuteTableParamedProcedure<T>(string storedProcedureName, string parameterName, string tableParamTypeName, IEnumerable<T> sprocParamObjects, SqlConnection connection = null)
 {
 SqlConnection adHocConnection = null;
 if (connection == null)
 {
 connection = new SqlConnection(connectionString);
 connection.Open();
 adHocConnection = connection;
 }
 using (adHocConnection)
 {
 using (SqlCommand command = connection.CreateCommand())
 {
 command.CommandText = storedProcedureName;
 command.CommandType = CommandType.StoredProcedure;
 SqlParameter parameter = command.Parameters.AddWithValue(parameterName, CreateDataTable(sprocParamObjects));
 parameter.SqlDbType = SqlDbType.Structured;
 parameter.TypeName = tableParamTypeName;
 command.ExecuteNonQuery();
 }
 }
 }
}

Naming
On the whole, your naming is just fine. I wonder, though, why you called your class GenericDataModel? What makes it generic? Can't it be called DataModel?

Comments
Since your method naming and parameter naming are descriptive - most of your comments are completely redundant - I don't need a comment to tell me that CallSprocsInTransaction call sprocs in transaction, or that sprocsToCall is a list of sprocs to call... These comments just add clutter and nothing else.
This also goes for inline comments, which tell us trivial things like you are creating the command you are going to send.

Potential leak - not using using
When you create the connection inside the method, you Close it at the end of the method, but you do not count for possible Exceptions inside the method. In such a case - the connection will not be closed.
To prevent it you can use try...finally, but you can also take advantage of the fact that using supports null objects:

public void ExecuteTableParamedProcedure<T>(string storedProcedureName, string parameterName, string tableParamTypeName, IEnumerable<T> sprocParamObjects, SqlConnection connection = null)
 {
 SqlConnection adHocConnection = null;
 if (connection == null)
 {
 connection = new SqlConnection(connectionString);
 connection.Open();
 adHocConnection = connection;
 }
 using (adHocConnection)
 {
 using (SqlCommand command = connection.CreateCommand())
 {
 command.CommandText = storedProcedureName;
 command.CommandType = CommandType.StoredProcedure;
 SqlParameter parameter = command.Parameters.AddWithValue(parameterName, CreateDataTable(sprocParamObjects));
 parameter.SqlDbType = SqlDbType.Structured;
 parameter.TypeName = tableParamTypeName;
 command.ExecuteNonQuery();
 }
 }
 }
}

Naming
On the whole, your naming is just fine. I wonder, though, why you called your class GenericDataModel? What makes it generic? Can't it be called DataModel?

Comments
Since your method naming and parameter naming are descriptive - most of your comments are completely redundant - I don't need a comment to tell me that CallSprocsInTransaction call sprocs in transaction, or that sprocsToCall is a list of sprocs to call... These comments just add clutter and nothing else.
This also goes for inline comments, which tell us trivial things like you are creating the command you are going to send.

Potential leak - not using using
When you create the connection inside the method, you Close it at the end of the method, but you do not count for possible Exceptions inside the method. In such a case - the connection will not be closed.
To prevent it you can use try...finally, but you can also take advantage of the fact that using supports null objects:

public void ExecuteTableParamedProcedure<T>(string storedProcedureName, string parameterName, string tableParamTypeName, IEnumerable<T> sprocParamObjects, SqlConnection connection = null)
 {
 SqlConnection adHocConnection = null;
 if (connection == null)
 {
 connection = new SqlConnection(connectionString);
 connection.Open();
 adHocConnection = connection;
 }
 using (adHocConnection)
 {
 using (SqlCommand command = connection.CreateCommand())
 {
 command.CommandText = storedProcedureName;
 command.CommandType = CommandType.StoredProcedure;
 SqlParameter parameter = command.Parameters.AddWithValue(parameterName, CreateDataTable(sprocParamObjects));
 parameter.SqlDbType = SqlDbType.Structured;
 parameter.TypeName = tableParamTypeName;
 command.ExecuteNonQuery();
 }
 }
 }
}
Source Link
Uri Agassi
  • 6.7k
  • 1
  • 18
  • 48

Naming
On the whole, your naming is just fine. I wonder, though, why you called your class GenericDataModel? What makes it generic? Can't it be called DataModel?

Comments
Since your method naming and parameter naming are descriptive - most of your comments are completely redundant - I don't need a comment to tell me that CallSprocsInTransaction call sprocs in transaction, or that sprocsToCall is a list of sprocs to call... These comments just add clutter and nothing else.
This also goes for inline comments, which tell us trivial things like you are creating the command you are going to send.

Potential leak - not using using
When you create the connection inside the method, you Close it at the end of the method, but you do not count for possible Exceptions inside the method. In such a case - the connection will not be closed.
To prevent it you can use try...finally, but you can also take advantage of the fact that using supports null objects:

public void ExecuteTableParamedProcedure<T>(string storedProcedureName, string parameterName, string tableParamTypeName, IEnumerable<T> sprocParamObjects, SqlConnection connection = null)
 {
 SqlConnection adHocConnection = null;
 if (connection == null)
 {
 connection = new SqlConnection(connectionString);
 connection.Open();
 adHocConnection = connection;
 }
 using (adHocConnection)
 {
 using (SqlCommand command = connection.CreateCommand())
 {
 command.CommandText = storedProcedureName;
 command.CommandType = CommandType.StoredProcedure;
 SqlParameter parameter = command.Parameters.AddWithValue(parameterName, CreateDataTable(sprocParamObjects));
 parameter.SqlDbType = SqlDbType.Structured;
 parameter.TypeName = tableParamTypeName;
 command.ExecuteNonQuery();
 }
 }
 }
}
default

AltStyle によって変換されたページ (->オリジナル) /