问题  作为最佳实践的函数Exec​​uteNonQuery有什么问题?

已加入
2018年9月26日
留言内容
22
编程经验
3-5
我在C#应用程序上工作,我需要使功能进行插入数据或动态更新或删除,以便在下面执行插入功能

或更新或删除,但我不知道必须从下面的函数中添加或删除什么才能使函数成为最佳实践。
C#:
public static async Task<int> ExecuteNonQuery(string sql, SqlConnection sqlconnection, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
{
    int RecordsCount = 0;

    if (sql == "") return 0;
    
    await Task.Run(async () =>
                   {
                       using (var con = new SqlConnection(GlobalVariables.con))
                       {
                           using (var cmd = new SqlCommand() { Connection = con })
                           {
                               if (cmd.CommandTimeout < 360)
                                   cmd.CommandTimeout = 360;
                              
                               cmd.CommandText = sql;
                               cmd.CommandType = cmdType;
                               cmd.Parameters.Clear();
                              
                               if (@params != null)
                               {
                                   for (int i = 0; i < @params.Length; i++)
                                   {
                                       cmd.Parameters.Add(@params[i]);
                                   }
                               }
                              
                               try
                               {
                                   await con.OpenAsync();

                                   RecordsCount = (await cmd.ExecuteNonQueryAsync());
                               }
                               catch (Exception ex)
                               {
                                   throw new Exception(ex.Message);
                               }
                           }
                       }
                   });
    
    return RecordsCount;
}
所以我在上面做功能来进行插入,更新或删除

最佳做法还剩下什么还是错了?
 
由主持人最后编辑:

金西尼

C#论坛主持人
工作人员
已加入
2011年4月23日
留言内容
3,525
地点
悉尼,澳大利亚
编程经验
10+
  • 您正在传递 SqlConnection 但忽略它并创建一个新的。
  • 您有两个嵌套的using块,它们之间没有代码,因此您只需要一个,即不需要:
C#:
using (var x = new SomeType())
{
    using (var y = new SomeOtherType())
    {
        // ...
    }
}
用这个:
C#:
using (var x = new SomeType())
using (var y = new SomeOtherType())
{
    // ...
}
  • 您在这里没有充分利用对象初始化程序:
C#:
using (var cmd = new SqlCommand() { Connection = con })
{
    if (cmd.CommandTimeout < 360)
        cmd.CommandTimeout = 360;

    cmd.CommandText = sql;
    cmd.CommandType = cmdType;
您使用无参数构造函数,然后设置两个可以通过其他构造函数设置的属性,然后通过对象初始化程序设置一个属性,然后按常规设置三个属性。那 如果 考虑到您只是创建了命令对象并确切知道了什么,该语句也没有意义。 CommandTimeout 是。该代码应如下所示:
C#:
using (var cmd = new SqlCommand(sql, con) { CommandType = cmdType, CommandTimeout = 360 })
{
  • 清除目标有什么意义 参数 您刚刚创建的命令对象的集合,因此知道没有参数?
  • 参数 集合有一个 添加范围 方法,因此不需要循环。
  • 你在做什么 试着抓 坏的原因有很多。为什么要用同一条消息创建自己的异常,而不是让原始异常中包含的所有信息冒泡?如果您有创建自己的例外的正当理由,则应提供自己的消息并将原始例外设置为 内部异常. If you're not going to create your own exception but you have something else useful to do, e.g. log the exception, then you should just rethrow the original exception. You can do that using throw ex; but that's bad because it truncates the stack trace to the current method. To rethrow the original exception in its full glory, just use throw;. That said, 如果你什么都不做 抓住 阻塞但重新抛出异常,那么您根本就不应捕获异常,因为最终结果是相同的,而且您不会浪费捕获重新抛出的时间。
 

金西尼

C#论坛主持人
工作人员
已加入
2011年4月23日
留言内容
3,525
地点
悉尼,澳大利亚
编程经验
10+
是什么阻止您自己实施建议?一般来说,它应该起作用的方式是,您先为自己做些自己可以做的事,然后要求我们为您做不到或不能做的事情提供帮助。
 

跳伞

工作人员
已加入
2019年4月6日
留言内容
2,538
地点
弗吉尼亚州切萨皮克
编程经验
10+
Also, it the code above, it's unclear why you are doing a 任务运行 () to kick off an asynchronous task, when you calling an asynchronous method anyway. Is there a bug in the SQL driver where it is actually doing work synchronously even though you call the asynchronous method?
 
已加入
2018年9月26日
留言内容
22
编程经验
3-5
我还有其他问题,需要回答可能的话
使用超时有什么好处,我还需要删除。
Task.run我以前使用过,它没有给我任何问题
告诉我之后的最终方法如下,因此,如果您有任何意见,请告诉我
C#:
public  async Task<int> ExecuteNonQuery(string sql, SqlConnection sqlconnection, DbParameter[] @params = null, CommandType cmdType = CommandType.StoredProcedure)
    {
        int RecordsCount = 0;
      

            if (sql == "") return 0;
            await Task.Run(async () =>
            {
                using (sqlconnection = new SqlConnection(GlobalVariables.con))
                {
                    await sqlconnection.OpenAsync();
                    var insertTransaction = sqlconnection.BeginTransaction();
                    using (var cmd = new SqlCommand(sql,sqlconnection) {  Transaction = insertTransaction,CommandType=cmdType })
                    {
                  
                        cmd.CommandText = sql;
                        cmd.CommandType = cmdType;
                        cmd.Parameters.Clear();
                        if (@params != null && @params.Length > 0)
                        {
                            cmd.Parameters.AddRange(@params);

                        }
                        try
                        {
                            

                           RecordsCount = (await cmd.ExecuteNonQueryAsync());
                            insertTransaction.Commit();
                        }
                        catch (SqlException)
                        {
                            insertTransaction.Rollback();
                            throw;

                        }
                        catch (Exception)
                        {
                            throw;
                        }
                    }

                }
                

            });
        return RecordsCount;
    }
 

金西尼

C#论坛主持人
工作人员
已加入
2011年4月23日
留言内容
3,525
地点
悉尼,澳大利亚
编程经验
10+
使用超时有什么好处,我还需要删除。
超时是什么时间?如果您不知道,那么也许您应该阅读文档以进行查找。您可以先为自己寻找这些东西。我们将为您提供您无法自己做的事情。
Task.run我以前使用过,它没有给我任何问题
您以前曾经使用过某些东西并且它没有损坏,这不是再次使用它的好理由。目的是什么 任务运行 ?在这种情况下适用吗?如果没有,那么使用它不是一个好主意。
 

金西尼

C#论坛主持人
工作人员
已加入
2011年4月23日
留言内容
3,525
地点
悉尼,澳大利亚
编程经验
10+
您已尝试实施我的建议,对此我表示赞赏。但是,在某些情况下,您做得很差。第一点是:
You're passing in a SqlConnection 但忽略它并创建一个新的。
您已经更改了代码,但是仍然忽略了传入的连接对象。您仍在此处创建新的连接对象:
C#:
using (sqlconnection = new SqlConnection(GlobalVariables.con))
因此您将参数用作变量,但忽略了已分配的对象。我认为很明显,您只是在网上复制/粘贴代码,而没有花时间了解它的作用。确定连接:要在其中传递连接对象还是在方法中创建一个新对象?如果您想一次通过一个,那就创建一个新的。如果要创建一个新的,则完全删除该参数。考虑一下您想做什么,然后编写代码来做到这一点。
 

金西尼

C#论坛主持人
工作人员
已加入
2011年4月23日
留言内容
3,525
地点
悉尼,澳大利亚
编程经验
10+
还有这个:
您没有充分利用对象初始化程序
您已更改代码以使用更好的构造函数并更好地利用对象初始化程序:
C#:
using (var cmd = new SqlCommand(sql, sqlconnection) { Transaction = insertTransaction, CommandType=cmdType })
{
    cmd.CommandText = sql;
    cmd.CommandType = cmdType;
但是您仍然可以毫无意义地设置两个属性!每行代码都应有特定的用途。思考一下您在做什么以及为什么这么做。当构造函数已经设置了一个属性而另一个对象初始化器已经设置了另一个属性时,为什么还要设置这两个属性呢?如果您不知道构造函数在做什么,或者对象初始化程序在做什么,那么请停止编写您不了解并无法理解的代码。
 

金西尼

C#论坛主持人
工作人员
已加入
2011年4月23日
留言内容
3,525
地点
悉尼,澳大利亚
编程经验
10+
接下来,我们有这个:
清除目标有什么意义 参数 您刚刚创建的命令对象的集合,因此知道没有参数?
您完全忽略了它。
还有这个:
如果你什么都不做 抓住 阻止但抛出异常,那么您根本不应该捕获它
你有这个:
C#:
 抓住  (Exception)
{
    throw;
}
本质上这不是厌倦,只是毫无意义。不管是否存在,最终结果都是相同的;如果我没记错的话,最终效率会更高。
 

跳伞

工作人员
已加入
2019年4月6日
留言内容
2,538
地点
弗吉尼亚州切萨皮克
编程经验
10+
Also notice that the SqlTransaction is an IDisposable object. If you are taking time to dispose of the SqlCommand and the SqlConnection, why are you not disposing SqlTransaction?
 
最佳 底部