Mi dispiace solo per aver commentato in primo luogo, ma sto postando quasi ogni giorno un commento simile poiché molte persone pensano che sarebbe intelligente incapsulare la funzionalità ADO.NET in una classe DB (anche io 10 anni fa). Per lo più decidono di utilizzare oggetti statici/condivisi poiché sembra essere più veloce che creare un nuovo oggetto per qualsiasi azione.
Non è una buona idea né in termini di prestazioni né in termini di sicurezza contro i guasti.
Non fare il bracconaggio sul territorio del Connection-Pool
C'è una buona ragione per cui ADO.NET gestisce internamente le connessioni sottostanti al DBMS nel pool di connessioni ADO-NET:
In pratica, la maggior parte delle applicazioni utilizza solo una o poche diverse configurazioni per le connessioni. Ciò significa che durante l'esecuzione dell'applicazione, molte connessioni identiche verranno aperte e chiuse ripetutamente. Per ridurre al minimo i costi di apertura delle connessioni, ADO.NET utilizza una tecnica di ottimizzazione denominata pool di connessioni.
Il pool di connessioni riduce il numero di volte in cui è necessario aprire nuove connessioni. Il pooler mantiene la proprietà della connessione fisica. Gestisce le connessioni mantenendo in vita un insieme di connessioni attive per ogni configurazione di connessione data. Ogni volta che un utente chiama Open su una connessione, il pooler cerca una connessione disponibile nel pool. Se è disponibile una connessione in pool, la restituisce al chiamante invece di aprire una nuova connessione. Quando l'applicazione chiama Close sulla connessione, il pooler la restituisce all'insieme di connessioni attive in pool invece di chiuderlo. Una volta che la connessione è tornata al pool, è pronta per essere riutilizzata alla successiva chiamata Open.
Quindi ovviamente non c'è motivo per evitare di creare, aprire o chiudere connessioni poiché in realtà non vengono create, aperte e chiuse affatto. Questo è "solo" un flag per il pool di connessioni per sapere quando una connessione può essere riutilizzata o meno. Ma è un flag molto importante, perché se una connessione è "in uso" (presuppone il pool di connessioni), una nuova connessione fisica deve essere aperta al DBMS, cosa molto costosa.
Quindi non stai ottenendo un miglioramento delle prestazioni ma il contrario. Se viene raggiunta la dimensione massima del pool specificata (100 è l'impostazione predefinita), otterresti anche eccezioni (troppe connessioni aperte ...). Quindi questo non solo avrà un enorme impatto sulle prestazioni, ma sarà anche una fonte di brutti errori e (senza utilizzare Transazioni) un'area di dumping dei dati.
Se stai usando anche connessioni statiche, stai creando un blocco per ogni thread che tenta di accedere a questo oggetto. ASP.NET è un ambiente multithread per natura. Quindi c'è una grande possibilità per questi blocchi che causano nella migliore delle ipotesi problemi di prestazioni. In realtà prima o poi riceverai molte eccezioni diverse (come il tuo ExecuteReader richiede una connessione aperta e disponibile ).
Conclusione :
- Non riutilizzare affatto le connessioni o qualsiasi oggetto ADO.NET.
- Non renderli statici/condivisi (in VB.NET)
- Crea, apri (in caso di Connessioni), usa, chiudi e smaltiscili sempre dove ne hai bisogno (es. in un metodo)
- usa la
using-statementdi disporre e chiudere (in caso di Connessioni) implicitamente
Questo è vero non solo per Connections (sebbene il più evidente). Ogni oggetto che implementa IDisposable dovrebbe essere eliminato (più semplice da using-statement ), tanto più in System.Data.SqlClient spazio dei nomi.
Tutto quanto sopra parla contro una classe DB personalizzata che incapsula e riutilizza tutti gli oggetti. Questo è il motivo per cui ho commentato di cestinarlo. Questa è solo una fonte di problemi.
Modifica :Ecco una possibile implementazione della tua retrievePromotion -metodo:
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;
}
