0

as a developer with a PHP background I've always separated the connection to DB and the Query in 2 separate functions. The logic is why should I call Connect(); every time I want to query to the DB? Isn't that costly?

Now I'm working on a project with C# and SQL Server 2016 and came across the following sample from MSDN.

What I noticed is that they do both the connection and the query in a single method. That looked strange to me as I said I'm a PHP developer, so a few questions:

  1. Even though the method is asynchronous, wouldn't calling every time I need to query something be costly because it calls SqlConnection.OpenAsync();?
  2. Why aren't they closing the connection when they are done?
  3. Is separating the DB connection and the query a good practice in C#? If so, how would you advise me to do it? And if not, why, and what is an efficient way to connect to and manage a database?

Here is my attempt (untested) to separate connection and query:

private SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder();
private Task<SqlConnection> connection;

private async Task<SqlConnection> ConnectDB()
{
    // Build connection string
    builder.DataSource = "localhost";
    builder.UserID = "user";
    builder.Password = "pass";
    builder.InitialCatalog = "test";

    // Connect to SQL
    SqlConnection connection = new SqlConnection(builder.ConnectionString);
    await connection.OpenAsync();
    return connection;
}

private async void Query(string cmd)
{
    if (cmd == string.Empty || cmd == null)
    {
        throw new ArgumentException("Query string is empty or null.");
    }

    SqlCommand command = new SqlCommand(cmd, connection.Result);
    await command.ExecuteNonQueryAsync();
}

// Main Form Constructor
public MainForm()
{
    connection = ConnectDB();
}

// Example usage?
private void GetAll()
{
    Query("SELECT * FROM `test`");
}
Johny P.
  • 648
  • 1
  • 9
  • 31

2 Answers2

1

1) Regarding only your fist question. ADO.NET keeps a pool of connections, therefore as long as the connection strings do not vary the performance hit to opening new connections is not that bad.

ADO.NET Connection Pooling

2) After reading the sample whoever wrote it is utilizing the "using" keyword as a using statement. When used this way the object wrapped in the using keyword will be disposed (closed) once it goes out of scope. Note, the using keyword has two purposes in C#. They're now both linked below just note you're interested in the second link.

C# Using Directive

C# Using Statement

3) I prefer to keep the connection and the queries together. I believe that is considered best practice. I wouldn't necessarily say it is bad practice to not, but I personally would avoid it.

Reasons being keeping a database connection open longer then it needs to be alive is frowned upon.

Lets say you have a class which has a SqlConnection, SqlCommand and SqlDataReader property/field instances. If for whatever reason you need to fire a command while another command is already running you'll have to open a new SqlDataReader to perform the other query because the underlying class components cannot handle multiple queries at once. The only real issue here is the SqlDataReader. You could theoretically get around that by just creating instances of that type when needed and keeping one SqlCommand and SqlConnection instance within the class, but I would advice not to do that.

Finally, if you kept class scoped instances your class needs to implement IDisposable and handle disposing all of these objects. Whereas, if all the objects were properly disposed in each method there would be no need.

Its just simply easier to avoid keeping properties/field instances of these types within a class.

(Sorry I phrased #3 poorly to begin with)

580
  • 470
  • 2
  • 12
  • Yeah, I didn't know `Dispose()` = `Close()`, as is explained in the Connection Pooling article you linked. But I haven't thought about the case you wrote in 3). So I guess I will do the connection and queries together. I didn't know about connection pooling, so that's fine. Thanks! – Johny P. Jun 14 '18 at 21:43
  • Read up on [IDisposable](https://msdn.microsoft.com/en-us/library/system.idisposable(v=vs.110).aspx). Disposing doesn't always exactly mean close. For some types (Which I cannot recall off-hand) there are other methods that need to be called or called in addition to dispose. So just be aware of that. – 580 Jun 14 '18 at 21:47
  • As an aside this may be helpful to you as well [Dispose Pattern](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/dispose-pattern) – 580 Jun 14 '18 at 21:49
0

1) As mentioned, .NET keeps a connection pool and will re-use the connections behind the scenes to reduce the overhead of reconnecting to the database every time. Connection Pooling

2) In the sample code, when the code exits the using block, Dispose will be called on the connection object, which automatically closes it. See this question for more info.

3) Personally, I prefer keeping the connection and query together; separating them offers little to no performance benefit and makes it harder to ensure that the connection is closed and disposed of properly when you're finished with it.

nschneid
  • 1
  • 1