Rod
Rod

Reputation: 15433

Can I consolidate this in a generic method?

I have a DataAccessBase class with the following 2 data access methods. One for ExecuteScalar and one for ExecuteNonQuery. Is it possible to consolidate it into one generic method or is even worth worrying about?

    protected static int ExecuteNonQuery(SqlCommand command)
    {
        using (SqlConnection connection = new SqlConnection(_connStr))
        {
            command.Connection = connection;
            SqlDataAdapter da = new SqlDataAdapter(command);
            command.Connection.Open();

            int result = command.ExecuteNonQuery();

            return result;
        }
    }

    protected static string ExecuteScalar(SqlCommand command)
    {
        using (SqlConnection connection = new SqlConnection(_connStr))
        {
            command.Connection = connection;
            SqlDataAdapter da = new SqlDataAdapter(command);
            command.Connection.Open();

            string result = command.ExecuteScalar().ToString();

            return result;
        }
    }

    private static DataTable GetDT(int id)
    {
        using (SqlConnection connection = new SqlConnection(_connStr))
        {
            string query = "select id, userid, name from tasks where id = @id";
            SqlCommand command = new SqlCommand(query, connection);
            SqlDataAdapter da = new SqlDataAdapter(command);
            //Parameterized query to prevent injection attacks
            command.Parameters.AddWithValue("id", id);
            DataTable dt = new DataTable();
            da.Fill(dt);

            return dt;
        }
    }

Upvotes: 1

Views: 711

Answers (2)

Jon Skeet
Jon Skeet

Reputation: 1499860

You can definitely avoid the current repetition you've got with a generic method, but I wouldn't try to reduce it to a single method. Here's what I'd potentially do:

protected static int ExecuteNonQuery(SqlCommand command) =>
    ExecuteCommand(command, cmd => cmd.ExecuteNonQuery());

protected static string ExecuteScalar(SqlCommand command) =>
    ExecuteCommand(command, cmd => cmd.ExecuteScalar().ToString());

private static T ExecuteCommand<T>(SqlCommand command, Func<SqlCommand, T> resultRetriever)
{
    using (SqlConnection connection = new SqlConnection(_connStr))
    {
        command.Connection = connection;
        command.Connection.Open();
        return resultRetriver(command);
    }
}

For the DataTable one, following the same pattern you'd create the command first:

protected static DataTable GetDataTable(SqlCommand command) =>
    ExecuteCommand(cmd =>
    {
        SqlDataAdapter da = new SqlDataAdapter(cmd)
        DataTable table = new DataTable();
        da.FillTable(table);
        return table;
    });

Upvotes: 7

JohnLBevan
JohnLBevan

Reputation: 24410

You can convert the ExecuteScalar to a generic method, allowing you to change the return type.

public T ExecuteScalar<T>(SqlCommand command)
{
    using (SqlConnection connection = new SqlConnection(_connStr))
    {
        command.Connection = connection;
        //SqlDataAdapter da = new SqlDataAdapter(command); //not needed...
        command.Connection.Open();
        var result = command.ExecuteScalar();

        //rather than just returning result with an implicit cast, use Max's trick from here: https://stackoverflow.com/a/2976427/361842
        if (Convert.IsDbNull(result))
            return default(T); //handle the scenario where the returned value is null, but the type is not nullable (or remove this to have such scenarios throw an exception)
        if (result is T)
            return (T)result;
        else
            (T)Convert.ChangeType(result, typeof(T));
    }
}

The logic is different in this method to in the ExecuteNonQuery function though, so you cannot have both represented by the same method.


Update

Regarding your question about the data table, I've taken and adapted @JonSkeet's answer to allow the class to also handle data tables:

public class SqlDatabaseThing //: ISqlDatabaseThing
{

    // ... additional code here ... //

    public int ExecuteNonQuery(SqlCommand command, IEnumerable<SqlParameter> sqlParameters = new[]{}) =>
        ExecuteNonQuery(_connStr, command, sqlParameters);
    public static int ExecuteNonQuery(string connectionString, SqlCommand command, IEnumerable<SqlParameter> sqlParameters = new[]{}) =>
        ExecuteCommand(connectionString, command, cmd => cmd.ExecuteNonQuery());

    public T ExecuteScalar(SqlCommand command, IEnumerable<SqlParameter> sqlParameters = new[]{}) =>
        ExecuteScalar(_connStr, command, sqlParameters);
    public static T ExecuteScalar(string connectionString, SqlCommand command, IEnumerable<SqlParameter> sqlParameters = new[]{}) =>
        ExecuteCommand(connectionString, command, cmd => ConvertSqlCommandResult(cmd.ExecuteScalar()));

    public DataTable ExecuteToDataTable(SqlCommand command, IEnumerable<SqlParameter> sqlParameters = new[]{}) =>
        ExecuteToDataTable(_connStr, command, sqlParameters);
    public static DataTable ExecuteToDataTable(string connectionString, SqlCommand command, IEnumerable<SqlParameter> sqlParameters = new[]{}) =>
        ExecuteCommand(connectionString, command, cmd => PopulateDataTable(cmd));


    private static T ExecuteCommand<T>(string connectionString, SqlCommand command, IEnumerable<SqlParameter> sqlParameters, Func<SqlCommand, T> resultRetriever)
    {
        using (SqlConnection connection = new SqlConnection(connectionString))
        {
            command.Parameters.AddRange(sqlParameters);
            command.Connection = connection;
            command.Connection.Open();
            return resultRetriver(command);
        }
    }

    private static DataTable PopulateDataTable(SqlCommand command)
    {
        var da = SqlDataAdapter(command);
        var dt = new DataTable();
        da.Fill(dt);
        return dt;
    }

    private static T ConvertSqlCommandResult(object result)
    {
        if (Convert.IsDbNull(result))
            return default(T); 
        if (result is T)
            return result as T;
        (T)Convert.ChangeType(result, typeof(T));
    }

}   

NB: In your code you'd included logic related to getting specific tasks. That should be kept separate from your generic database logic (i.e. as presumably you'll want to return data tables for various queries, and don't want to have to rewrite your GetDT code each time). As such I've provided additional sample code below showing how you could separate that logic into another class...

public class TaskRepository //: IRepository<Task>
{
    ISqlDatabaseThing db;
    public TaskRepository(ISqlDatabaseThing db)
    {
        this.db = db;
    }

    readonly string GetByIdCommand = "select id, userid, name from tasks where id = @id";
    readonly string GetByIdCommandParameterId = "@id"
    readonly SqlDbType GetByIdCommandParameterIdType = SqlDbType.BigInt;
    public Task GetById(long id)
    {
        var command = new SqlCommand(GetByIdCommand);
        var parameters = IEnumerableHelper.ToEnumerable<SqlParameter>(new SqlParameter(GetByIdCommandIdParameter, GetByIdCommandIdParameterType, id));
        var dataTable = db.ExecuteToDataTable(command, parameters);
        return DataTableToTask(dataTable)[0];
    }
    private IEnumerable<Task> DataTableToTask(DataTable dt)
    {
        foreach (var row in dt.Rows)
        {
            yield return DataRowToTask(row);
        }
    }
    private Task DataRowToTask (DataRow dr)
    {
        return new Task()
        {
            Id = dr["Id"]
            ,Name = dr["Name"]
            ,UserId = dr["UserId"]
        };
    }

}

public static class IEnumerableHelper
{
    public static IEnumerable<T> ToEnumerable<T>(params T[] parameters)
    {
        return parameters;
    } 
}

NB: This code is untested; any issues please let me know.

Upvotes: 1

Related Questions