In case of this method:
public void Delete(int id)
{
using (var connection = GetOpenConnection())
{
connection.Execute($"DELETE FROM MyTable WHERE Id = {id}");
}
}
Or just:
GetOpenConnection().Execute($"DELETE FROM MyTable WHERE Id = {id}");
I wonder if the second is the best option to ease the maintenance and simplify.
Answering this requires an understanding of how Sql Server (and other databases) use connections, and how ADO.Net uses connection pooling.
Database servers tend to only be able to handle a limited a number of active connections at a time. It has partly to do with the limited available ephemeral ports on a system, but other factors can come into play, as well. This is means it's important to make sure connections are either always closed promptly, or that we carefully limit connection use. If we want a database to scale to a large number of users, we have to do both.
.Net addresses this situation in two ways. First, the ADO.Net library you use for database access (System.Data
and company) includes a feature called Connection Pooling. This feature pools and caches connections for you, to make it efficient to quickly open and close connections as needed. The feature means you should not try to keep a shared connection object active for the life of an application or session. Let the connection pool handle this, and create a brand new connection object for most trips to the database.
The other way it addresses the issue is with the IDisposable pattern. IDisposable
provides an interface with direct support in the runtime via the using
keyword, such that you can be sure unmanaged resources for an object — like that ephemeral port on the database server your connection was holding onto — are cleaned up promptly and in a deterministic way, even if an exception is thrown. This feature makes sure all those short-lived connections you create because of the connection pooling feature really are as short-lived as they need to be.
In other words, the using block in the first sample serves an important function. It's a mistake to omit it. On a busy system it can even lead to a denial of service situation for your database.
You get a sense of this in the question title itself, which asks, "Which is better to dispose the object?" Only one of those two samples disposes the object at all.
First option gives you predictability: connection object returned from GetOpenConnection()
will be disposed as soon as connection.Execute
finishes.
On the other hand, if you use second approach, you can hope that the connection would be closed at some time in the future, but you have absolutely no certainty of when, and even if, it is going to happen.
Therefore one should prefer the first approach.
Note: Consider parameterizing your query. Even though in your situation insertion of the id
into the query is non-threatening because id
's type is int
, it is a good idea to use parameters consistently throughout your code.