Förlåt för att jag bara kommenterar i första hand, men jag postar nästan varje dag en liknande kommentar eftersom många tror att det skulle vara smart att kapsla in ADO.NET-funktionalitet i en DB-klass (jag också för 10 år sedan). Oftast bestämmer de sig för att använda statiska/delade objekt eftersom det verkar vara snabbare än att skapa ett nytt objekt för någon åtgärd.
Det är varken en bra idé när det gäller prestanda eller när det gäller felsäkerhet.
Pochera inte på Connection-Pools territorium
Det finns en god anledning till varför ADO.NET internt hanterar de underliggande anslutningarna till DBMS i ADO-NET Connection-Pool:
I praktiken använder de flesta applikationer bara en eller några få olika konfigurationer för anslutningar. Detta innebär att många identiska anslutningar kommer att öppnas och stängas upprepade gånger under applikationskörning. För att minimera kostnaden för att öppna anslutningar använder ADO.NET en optimeringsteknik som kallas anslutningspooling.
Anslutningspooling minskar antalet gånger som nya anslutningar måste öppnas. Poolaren behåller ägandet av den fysiska anslutningen. Den hanterar anslutningar genom att hålla liv i en uppsättning aktiva anslutningar för varje given anslutningskonfiguration. Närhelst en användare ringer Öppna på en anslutning, letar poolaren efter en tillgänglig anslutning i poolen. Om en poolad anslutning är tillgänglig returneras den till den som ringer istället för att öppna en ny anslutning. När applikationen anropar Close på anslutningen, returnerar poolaren den till den poolade uppsättningen aktiva anslutningar istället för att stänga den. När anslutningen har återställts till poolen är den redo att återanvändas vid nästa öppna samtal.
Så uppenbarligen finns det ingen anledning att undvika att skapa, öppna eller stänga anslutningar eftersom de faktiskt inte skapas, öppnas och stängs alls. Detta är "bara" en flagga för anslutningspoolen för att veta när en anslutning kan återanvändas eller inte. Men det är en mycket viktig flagga, för om en anslutning "används" (anslutningspoolen antar), måste en ny fysisk anslutning vara öppen till DBMS, vilket är väldigt dyrt.
Så du får ingen prestandaförbättring utan tvärtom. Om den angivna maximala poolstorleken (100 är standard) nås, skulle du till och med få undantag (för många öppna anslutningar ...). Så detta kommer inte bara att påverka prestandan enormt utan också vara en källa till otäcka fel och (utan att använda Transaktioner) ett datadumpningsområde.
Om du till och med använder statiska anslutningar skapar du ett lås för varje tråd som försöker komma åt detta objekt. ASP.NET är en multithreading-miljö av naturen. Så det finns en stor chans för dessa lås som i bästa fall orsakar prestandaproblem. Förr eller senare kommer du faktiskt att få många olika undantag (som din ExecuteReader kräver en öppen och tillgänglig anslutning ).
Slutsats :
- Återanvänd inte anslutningar eller några ADO.NET-objekt alls.
- Gör dem inte statiska/delade (i VB.NET)
- Skapa, öppna (vid anslutningar), använd, stäng och kassera dem alltid där du behöver dem (t.ex. i en metod)
- använd
using-statement
att kassera och stänga (vid anslutningar) implicit
Det gäller inte bara för anslutningar (även om det är mest märkbart). Varje objekt som implementerar IDisposable
bör kasseras (enklast genom att using-statement
), desto mer i System.Data.SqlClient
namnutrymme.
Allt ovanstående talar emot en anpassad DB-klass som kapslar in och återanvänder alla objekt. Det är anledningen till att jag kommenterade för att skräpa det. Det är bara en problemkälla.
Redigera :Här är en möjlig implementering av din retrievePromotion
-metod:
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 [email protected]";
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;
}