0

この形式のデータベース コマンド/接続を使用する場合に典型的な問題があるかどうか疑問に思っています。「より良い」ものはありますか?TSQL/C# のスキルを向上させるのに役立つ可能性のあるその他の情報をいただければ幸いです。ありがとうございました!

private void Approval_Status(object sender, EventArgs e)
    {
        Button Approval = (Button)sender;
        /*
         * Boolean determining if the request was approved or denied
         */
        Boolean Status = false;

        if (ValidateApproval(Approval.Text.Trim().ToUpper()) == true)
        {
            SqlCommand cmd0 = new SqlCommand();
            cmd0.Connection = db.con(user.Authority);
            cmd0.CommandType = CommandType.Text;
            cmd0.CommandText = "UPDATE [TBL_REQUEST] " +
                "SET [TBL_REQUEST].[REQUEST_STATUS]=@Status, [TBL_REQUEST].[APPROVED_BY]=@Approver, " +
                "[TBL_REQUEST].[DATE_APPROVED]=@Date, [TBL_REQUEST].[PRINTED_NAME]=@Name, " +
                "[TBL_REQUEST].[TITLE]=@Title, [TBL_REQUEST].[PTO_USED]=@Used " +
                "WHERE [TBL_REQUEST].[ID]=@ID; ";
            if (Approval.Text.ToUpper() == codes.RequestApproved)
            {
                cmd0.Parameters.AddWithValue("@Status", SqlDbType.VarChar).Value = codes.RequestApproved;
                Status = true ;
            }
            else
            {
                cmd0.Parameters.AddWithValue("@Status", SqlDbType.VarChar).Value = codes.RequestDenied;
                Status = false;
            }
            cmd0.Parameters.AddWithValue("@Approver", SqlDbType.VarChar).Value = user.User;
            cmd0.Parameters.AddWithValue("@Date", SqlDbType.Date).Value = DateTime.Today.ToShortDateString();
            cmd0.Parameters.AddWithValue("@Name", SqlDbType.VarChar).Value = txtApproval.Text.Trim();
            cmd0.Parameters.AddWithValue("@Title", SqlDbType.VarChar).Value = user.Title;
            cmd0.Parameters.AddWithValue("@Used", SqlDbType.Float).Value = (float)nudUsed.Value;
            cmd0.Parameters.AddWithValue("@ID", SqlDbType.VarChar).Value = txtID.Text.Trim();
            /*
             * Execute our non-query
             */
            db.conEstablished.Open();
            cmd0.ExecuteNonQuery();
            db.conEstablished.Close();
            /*
             * Dispose our resources
             */
            cmd0.Dispose();
            ClearRequestsPanel();
            /*
             * Inform our user of a successful update
             */
            if (Status == true)
            {
                MessageBox.Show(msg.RequestApproved);
            }
            else if (Status == false)
            {
                MessageBox.Show(msg.RequestDenied);
            }
        }
4

3 に答える 3

4

独自の接続プールを作成しようとしているようです。そうしないでください:

  • usingすべての使い捨てリソースに対してステートメントを使用します。これには と が含まれSqlConnectionますSqlCommand。そうすれば、例外がスローされても、リソースは破棄されます
  • SqlConnectionデータベース操作ごとに新しいものを作成し、システムが実際のネットワーク接続のプールを管理できるようにします。

何が何であるかは明確db.con(...)ではありませんが、単一の接続を取得している可能性が非常に高いようです。つまり、このコードをマルチスレッド環境で安全に使用することはできません。を作成するためのヘルパー メソッドを用意しても問題ありませんが、毎回新しいものを作成し、操作が完了したときに破棄する必要があります。db.conEstablishedSqlConnection

さらに、次の .NET 命名規則と次のコードを開始する必要があります。

if (Status == true)
{
    MessageBox.Show(msg.RequestApproved);
}
else if (Status == false)
{
    MessageBox.Show(msg.RequestDenied);
}

...次のように書くとよいでしょう:

MessageBox.Show(Status ? msg.RequestApproved : msg.RequestDenied);
于 2013-01-02T20:17:49.807 に答える
2

usingSqlConnection のような使い捨てオブジェクトを処理する「適切な」方法です。

コードの問題の一部は、クエリが例外を引き起こした場合、例外がメソッドから抜け出すため、この行がスキップされることです。

cmd0.Dispose();

dispose を使用usingすると、例外がブロックを終了しても、常に呼び出されます (内部的には、コードを a でラップしtry/catch、呼び出しを.Dispose()catch ブロックに入れます)。

SqlConnectionまた、クラスが内部でプーリングを処理することに注意してください。実際には、DB への単一のオープン ネットワーク接続ではありません。

于 2013-01-02T20:23:06.293 に答える
0

コードは次のようになります。明らかに、ここには他の場所で定義されている変数がありますが、うまくいけば、これが使用方法のより良いアイデアを提供しますusing

using(var dbconn = new SqlConnection(connectionString))
{
    using (var dbcmd = new SqlCommand(storedProcedure, dbconn))
    {
        dbcmd.CommandType = CommandType.StoredProcedure;
        dbcmd.Parameters.AddRange(sqlParameters.ToArray());
        dbconn.Open();
        return dbcmd.ExecuteNonQuery();
    }
}
于 2013-01-03T10:54:51.437 に答える