Sorry for commenting in the first place, but I post a similar comment almost every day, as many people think it would be wise to encapsulate the ADO.NET functionality in DB-Class (I, too, 10 years ago), They mainly decide to use static / general objects, because they seem to be faster than creating a new object for any action.
This is not a good idea in terms of efficiency, nor in terms of security during a breakthrough.
Do not use Connection-Pool panel
There is a good reason why ADO.NET internally manages the underlying DBMS Communications in the ADO-NET Connection-Pool :
In practice, most applications use only one or more different configurations for connections. This means that during application execution, many identical connections will reopen and close. To minimize the cost of opening connections, ADO.NET uses an optimization method called the connection pool.
The connection pool reduces the number of new connections that must be opened. The machine gun retains ownership of the physical connection. It manages connections, maintaining a live set of active connections for each connection configuration. Whenever a user calls Open when connected, the pool searches for an available connection in the pool. If a merged connection is available, it returns it to the caller instead of opening a new connection. When application calls close the connection, the pool will return it to the combined set of active connections instead of closing it. Once the connection returns to the pool, it is ready to be reused on the next Open call.
Thus, it is obvious that there is no reason not to create, open or close connections, since in fact they are not created, open and closed at all. This is a "only" flag for the connection pool, which should know when the connection can be reused. But this is a very important flag, because if the connection is “used” (the connection pool assumes), the new physical connection must be open to the DBMS, which is very expensive.
So, you are not improving performance, but vice versa. If the maximum pool size is reached (100 by default), you will even get exceptions (too many open connections ...). Thus, this will not only greatly affect performance, but also become a source of unpleasant errors and (without the use of transactions) areas of data dumping.
Even if you use static connections, you create a lock for each thread trying to access this object. ASP.NET is a multi-threaded environment by nature. Therefore, they have a great chance for these locks, which at best causes performance problems. In fact, sooner or later you will get many different exceptions (for example, ExecuteReader requires an open and accessible connection).
Conclusion :
- Do not reuse connections or any ADO.NET objects.
- Do not make them static / shared (in VB.NET)
- Always create, open (in case of connections), use, close and delete them where you need them (for example, in a method).
- use
using-statement to place and close (in case of connections) implicitly
This is true not only for Connections (although most notably). Each object that implements IDisposable must be deleted (the simplest using-statement ), especially in the System.Data.SqlClient namespace.
All of the above speaks against a custom DB class that encapsulates and reuses all objects. This is the reason why I commented on it. This is only a problem.
Change Here it is possible to execute your retrievePromotion method:
public Promotion retrievePromotion(int promotionID) { Promotion promo = null; var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MainConnStr"].ConnectionString; using (SqlConnection connection = new SqlConnection(connectionString)) { var queryString = "SELECT PromotionID, PromotionTitle, PromotionURL FROM Promotion WHERE PromotionID=@PromotionID"; using (var da = new SqlDataAdapter(queryString, connection)) { // you could also use a SqlDataReader instead // note that a DataTable does not need to be disposed since it does not implement IDisposable var tblPromotion = new DataTable(); // avoid SQL-Injection da.SelectCommand.Parameters.Add("@PromotionID", SqlDbType.Int); da.SelectCommand.Parameters["@PromotionID"].Value = promotionID; try { connection.Open(); // not necessarily needed in this case because DataAdapter.Fill does it otherwise da.Fill(tblPromotion); if (tblPromotion.Rows.Count != 0) { var promoRow = tblPromotion.Rows[0]; promo = new Promotion() { promotionID = promotionID, promotionTitle = promoRow.Field<String>("PromotionTitle"), promotionUrl = promoRow.Field<String>("PromotionURL") }; } } catch (Exception ex) { // log this exception or throw it up the StackTrace // we do not need a finally-block to close the connection since it will be closed implicitely in an using-statement throw; } } } return promo; }