-1
\$\begingroup\$

I am writing a C# Database connection class and I am trying to adhere to best practices and produce clean code.

In the CreateAndOpenDatabaseConnection function below I am instantiating the connection object and opening the connection and in the CreateSqlCommand function I am instantiating the command. Should the SqlConnection and the SqlCommand be class fields instead of being passed around?


internal static class DatabaseConnector
{
    private static SqlConnection CreateAndOpenDatabaseConnection()
    {
        string connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;
        SqlConnection connection = new SqlConnection(connectionString);
        connection.Open();
        return connection;
    }

    private static SqlCommand CreateSqlCommand(string query, SqlConnection connection)
    {
        return new SqlCommand(query, connection);
    }

    private static SqlDataReader CreateDataReader(SqlCommand command)  
    {  
        SqlDataReader dataReader = command.ExecuteReader();  
        return dataReader;  
    }          

    public static List<PlaceHolder> GetPlaceHolders()
    {
        var PlaceHolderList = new List<PlaceHolder>();

        using (SqlConnection connection = CreateAndOpenDatabaseConnection())
        {
            string query = {query};

            using (SqlDataReader reader = CreateDataReader(CreateSqlCommand(query, connection)))
            {
                while (reader.Read())
                {
                    PlaceHolderList.Add(new PlaceHolder {
                    });
                }
            }
        }
        return PlaceHolderList;
    }
}

        
```
\$\endgroup\$
4
  • 2
    \$\begingroup\$ Is this related to your previous question? \$\endgroup\$
    – Mast
    Commented Mar 1 at 18:50
  • \$\begingroup\$ @Mast I believe, it's not because I am asking regarding the SqlCommand as well and I want to know if it's better to make them fields, but correct me if I am wrong. \$\endgroup\$ Commented Mar 1 at 19:09
  • \$\begingroup\$ Try writing unit tests for the code that uses this class of yours. Try using asynchrony with this class of yours. Then throw it out, realizing the futility of trying. \$\endgroup\$ Commented Mar 2 at 16:12
  • \$\begingroup\$ Do not write your own ORM. Use EF or Dapper. \$\endgroup\$
    – BCdotWEB
    Commented Mar 3 at 19:48

1 Answer 1

2
\$\begingroup\$

Should the SqlConnection and the SqlCommand be class fields instead of being passed around?

None of the above. Have a SqlAdapter object and compose it with the connection, command, et.al. Then your Data Access Layer code talks to the Adapter and only the Adapter; with the DAL having method parameters for swappable SQL components.

SqlAdapters are designed to work this way, in keeping with the preferred composition paradigm (vice inheritance); Pushing down, so to speak, composition details to lower level of abstraction; keep the highest level of DAL use talking to the highest level SqlAdapter abstraction.

Read MSDN. It is astounding how flexible and composible-in-detail the SQLAdapter framework is.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.