Database abstraction layer for multiple providers

There's been a lot of questions lately about database provider and repository design especially without entity framework or alike.

I thought I try myself to create a reusable framework for this kind of stuff. So let's begin ;-)

TL;DR - There's an example at the bottom

Core

The main part of the framework is the DbProvider class with a corresponding interface. It encapsulates the most repetitive common tasks like managing a connection, executing queries and adding parameters. There is one thing that this early version lacks namely sql injection protection. The parameters need to be sanitized later.

I used generics to make it strongly typed and avoid magic strings later (like table or column names). With this design it supports each provider's own data types.

```
public interface IDbProvider
{
int ExecuteNonQuery(string sql, Action parameters = null);
IEnumerable> ExecuteReader(
string sql,
Action parameters = null);
}

public abstract class DbProvider : IDbProvider
where TConnection : DbConnection
where TCommand : DbCommand
{
private readonly DbProviderFactory _dbProviderFactory;

protected DbProvider(
string providerInvariantName,
string connectionString,
Action> dbConfiguration)
{
_dbProviderFactory = DbProviderFactories.GetFactory(providerInvariantName);
var dbConfigurationBuilder = new DbConfigurationBuilder(connectionString);
dbConfiguration(dbConfigurationBuilder);
DbConfiguration = dbConfigurationBuilder.Build();
}

public DbConfiguration DbConfiguration { get; }

public abstract string ParameterPrefix { get; }

public int ExecuteNonQuery(string sql, Action parameters = null)
{
var parameterBuilder = new ParameterBuilder();
parameters?.Invoke(parameterBuilder);

if (string.IsNullOrWhiteSpace(sql))
{
throw new ArgumentException("Value may not be null or whitespace", "sql");
}

DbProvider

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 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 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 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.

StackExchange Code Review Q#138850, answer score: 4