Return to Answer

• 1

• 50.9k
• 5
• 83
• 177

###DbProvider<TConnection, TCommand, TData>

In the ExecuteNonQuery() method you are doing stuff before you check the passed in parameter sql for validness. You should place the validation at the top of the method.

public int ExecuteNonQuery(string sql, Action<ParameterBuilder> parameters = null)
{
 if (string.IsNullOrWhiteSpace(sql))
 {
 throw new ArgumentException("Value may not be null or whitespace", "sql");
 } 

If the connection couldn't be opened because the passed connectionstring is wrong or the DBMS couldn't be reached, the work which may be done by the passed in Action<ParameterBuilder> parameters will be unnecessary/lost. So placing this inside the using block may be better like so

 using (var connection = _dbProviderFactory.CreateConnection())
 {
 connection.ConnectionString = DbConfiguration.ConnectionString;
 connection.Open();
 
 var parameterBuilder = new ParameterBuilder();
 parameters?.Invoke(parameterBuilder);
 using (var command = connection.CreateCommand())
 {
 command.CommandType = CommandType.Text;
 AddParameters((TCommand)command, parameterBuilder.Tables);
 return command.ExecuteNonQuery();
 }
 }

Right now this method takes two parameters, hence having a third which takes a CommandType enum wouldn't hurt and you wouldn't need to change this if a stored procedure should be used.

This results in

public int ExecuteNonQuery(string sql, CommendType commandType, Action<ParameterBuilder> parameters = null)
{
 if (string.IsNullOrWhiteSpace(sql))
 {
 throw new ArgumentException("Value may not be null or whitespace", "sql");
 }
 using (var connection = _dbProviderFactory.CreateConnection())
 {
 connection.ConnectionString = DbConfiguration.ConnectionString;
 connection.Open();
 var parameterBuilder = new ParameterBuilder();
 parameters?.Invoke(parameterBuilder);
 using (var command = connection.CreateCommand())
 {
 command.CommandType = commandType;
 AddParameters((TCommand)command, parameterBuilder.Tables);
 return command.ExecuteNonQuery();
 }
 }
} 

In the ExecuteReader() method you don't check the validness of the sql parameter. You really should do this. The mentioned points about the ExecuteNonQuery() method apply to this method as well.