Adding new methods to GenericDataAccess

Question

I have this GenericDataAccess class to interact with database from my school. I want to ask for some suggestions and advice in how to to improve it and add new methods to it for future use and reference

public static class GenericDataAccess
{
    static GenericDataAccess()
    {

    }

    /// <summary>
    /// creates and prepares a new DbCommand object on a new connection 
    /// </summary>
    /// <param name="command"></param>
    /// <returns></returns>
    public static DataTable ExecuteSelectCommand(DbCommand command)
    {
        // The DataTable to be returned 
        DataTable table;

        try
        {
            //open connection 
            command.Connection.Open();
            //execute command & save the result in DataTable 
            DbDataReader reader = command.ExecuteReader();
            table = new DataTable();
            table.Load(reader);
        }
        catch (Exception ex)
        {
           Utilities.LogError(ex);
            throw;
        }
        finally
        {
            command.Connection.Close();
        }

        return table;
    }

    /// <summary>
    /// creates and prepares a new DbCommand object on a new connection
    /// </summary>
    /// <returns></returns>
    public static DbCommand CreateCommand()
    {

        //obtaine the database provider name 
        string dataProviderName = CompuData_ProjectManagerConfiguration.DbProviderName;
        //Obtain database connection string 
        string connectionString = CompuData_ProjectManagerConfiguration.DbconnctionString;
        //Create new Data Provider Factory 
        DbProviderFactory factory = DbProviderFactories.GetFactory(dataProviderName);
        //obtain a database specific connection object
        DbConnection conn = factory.CreateConnection();
        //set the connection string 
        conn.ConnectionString = connectionString;
        //create a database specific command object
        DbCommand comn = conn.CreateCommand();
        //set the command type to store proc 
        comn.CommandType = CommandType.StoredProcedure;

        //return the initialized command object
        return comn;
    }

    /// <summary>
    /// execute an update, delete, or insert command and return the number of affected rows
    /// </summary>
    /// <param name="command"></param>
    public static int ExecuteNonQuery(DbCommand command)
    {
        //the number of affected rows 
        int affectedRows = -1;
        // Execute the command making sure the connection gets closed in the end
        try
        {
            //open connction 
            command.Connection.Open();
            affectedRows = command.ExecuteNonQuery();
        }
        catch (Exception ex)
        {
            Utilities.LogError(ex);
            throw;
        }
        finally
        {
            command.Connection.Close();
        }

        return affectedRows;
    }


    // execute a select command and return a single result as a string
    public static string ExecuteScaler(DbCommand command)
    {
        // the value to be retuned 
        string value = "";

        // Execute the command making sure the connection gets closed in the end
        try
        {
            //open the connection of command 
            command.Connection.Open();
            //execute the command and get the number of affected rows

            value = command.ExecuteScalar().ToString();
        }
        catch (Exception ex)
        {
           Utilities.LogError(ex);
            throw;
        }
        finally
        {
            command.Connection.Close();
        }
        return value;
    }
}

Answer 1

First of all, there is no need to write a generic data access class. Use the Data Access Application Block. It gives you everything you need to access a database. Why reinvent the wheel. You can even download the source code to this if you want to see how Microsoft does it.

Having said that, if you are doing this just as an exercise then I see a couple things...

I am not a big fan of static classes (except for possibly simple "function" classes, classes which provide generic functions such as a function to convert unix timestamp to datetime). Making classes static makes it more difficult to unit test.

Along these lines of unit testing I would also recommend you implement an interface so you could can make unit testing easier through IOC / dependency injection

Your methods should only do one thing. Adding too much functionality into each method limits the flexibility of the caller. You should have a method that creates a connection, another that creates a command, and another that executes the command. Example, I would not open and close my connections within the "Execute..." methods because what if the caller wants to open the connection so they can execute multiple commands in the same connection. If you want to add methods that do multiple things then they should be separate from your generic data access class, and they should call your generic data access methods.

I do not see anything to handle transactions. This would be something you would want to add.