コードスニペット.NETでのデータベース呼び出しの改善

Question

以下のコードで問題のリストを取得し、修正できますか？

string ProgramID = HttpContext.Current.Session[CommonFunctions.myNGconnectSessionVars.ProgramId].ToString(); 
      SqlConnection con = new SqlConnection(ConfigurationManager.AppSettings["MyNGConnectDashBoardConnectionString"].ToString()); 
      SqlCommand cmd = new SqlCommand(); 
      DataSet ds = new DataSet(); 
      try 
      { 
       cmd.Connection = con; 
       cmd.CommandTimeout = 900; 
       cmd.CommandText = "dsb_GetSubscriptionDetailsForSubscriber_ForValidations"; 
       cmd.CommandType = CommandType.StoredProcedure; 
       cmd.Parameters.Clear(); 
       cmd.Parameters.AddWithValue("@GPCustomerID", strGPCustomerID); 
       cmd.Parameters.AddWithValue("@UserName", strUserName); 
       cmd.Parameters.AddWithValue("@ServerName", ConfigurationManager.AppSettings["ServerName"].ToString()); 
       //code changed by sushma 3/22/2011 as new tab added for reach 
       //cmd.Parameters.AddWithValue("@ProgramID", CommonFunctions.ProgramID); 
       cmd.Parameters.AddWithValue("@ProgramID", ProgramID); 
       cmd.Parameters.AddWithValue("@IsTeacher", blnIsTeacher); 
       // cmd.CommandTimeout = 0; 
       SqlDataAdapter da = new SqlDataAdapter(cmd); 
       da.Fill(ds); 
      } 
      catch (Exception ex) 
      { 
       throw ex; 
      } 
      finally 
      { 
       con.Close(); 
      } 

      return ds; 

Answer 1

いくつかの即時改善

使用using文ではなく、接続管理のために最後に/しようとしている

使用throwないthrow ex、いっそあなただけのキャッチを削除することができますし、再スロー

インスタンス化したばかりのコマンドでParameters.Clearに電話する必要はありません。

最後に、DB作業をたくさん行う場合は、軽量のORMを使用することをお勧めします。

Answer 2

私が見ることができる最大の問題は、あなたがこのような接続、コマンド、アダプタなどの使い捨てのオブジェクトを配置していないということです...ので、例外の場合には、あなたがリソースをリークする可能性があります。あなたのコードラップを修正するにはusingステートメント内のすべての使い捨てのリソース：あなたが悪いですスタックトレースを変更すること以外catch文で有益な何かをやっているようには見えませんよう

using (SqlConnection con = new SqlConnection(ConfigurationManager.AppSettings["MyNGConnectDashBoardConnectionString"].ToString())) 
using (SqlCommand cmd = con.CreateCommand()) 
{ 
    con.Open(); 
    cmd.CommandTimeout = 900; 
    cmd.CommandText = "dsb_GetSubscriptionDetailsForSubscriber_ForValidations"; 
    cmd.CommandType = CommandType.StoredProcedure; 
    cmd.Parameters.AddWithValue("@GPCustomerID", strGPCustomerID); 
    cmd.Parameters.AddWithValue("@UserName", strUserName); 
    cmd.Parameters.AddWithValue("@ServerName", ConfigurationManager.AppSettings["ServerName"].ToString()); 
    using (SqlDataAdapter da = new SqlDataAdapter(cmd)) 
    { 
     DataSet ds = new DataSet(); 
     da.Fill(ds); 
     return ds; 
    } 
} 

は、私はまた、try/catchブロックを削除しました。

Answer 3

除い：数百回と

strUserName

を呼び出したときに

ConfigurationManager.AppSettings [ "サーバー名"]

はたぶん少し遅くなりますあなたはハンガリー語nを使って解雇される過去10年間、頑張って落胆した時は？

そしてそれ：

cmd.CommandTimeout = 900;

ああ - いいえ。 900秒？あなたはそれが必要なあなたがそこで何をしていると思いますか？

cmd.Parameters.Clear（）;

ええ。このように、新しいオブジェクトに必要です。

DataSet ds = new DataSet（）;

一般的な質問ツールを除いて、データセットを使用する一般的な人は誰でも起きます。

私はリアルではありませんが、ここでは宿題をレビューしていません。私はむしろuselesをUSINGステートメントに行く代わりにtry/catchを使用しません。再スロー例外へ

ここ

Answer 4

それは主に一方の構築物だ、私はコメントを付ける：

• throw exを使用しないでください - それは、元のスタックトレース変更します。代わりにthrow;を使用してください。
• catchブロックを離します。あなたは、単にしようと、最終的には
• 2本使用conとcmd

他のいくつかの点についてのステートメント使用して使用することができます。

• 使用自明の変数名
• をバージョンに変更履歴を置きます制御システム（そしてそれをコメントから削除してください）
• ソース管理システムに、単にコメントアウトするのではなく古いコードを扱わせてください。
• キャッシュは、あなたのDB接続
• 使用SqlConnection.CreateCommand