如何改进这个可重用方法类

本文关键字：方法 何改进 | 更新日期: 2023-09-27 18:25:41

我有一个类，它包含填充DropDowns、返回DataSet、返回Scalar或简单地执行查询的方法。在我在StackOverflow的一篇旧帖子中，我提交了一个同类的错误代码。根据贡献者的建议，我改进了代码，想知道这个类是否适合在高并发环境中使用：

public sealed class reuse
{
    public void FillDropDownList(string Query, DropDownList DropDownName)
    {
        using (TransactionScope transactionScope = new TransactionScope())
        {
            using (SqlConnection con = new SqlConnection(ConfigurationManager.ConnectionStrings["MyDbConnection"].ConnectionString.ToString()))
            {
                SqlDataReader dr;
                try
                {
                    if (DropDownName.Items.Count > 0)
                        DropDownName.Items.Clear();
                    SqlCommand cmd = new SqlCommand(Query, con);
                    dr = cmd.ExecuteReader();
                    while (dr.Read())
                        DropDownName.Items.Add(dr[0].ToString());
                    dr.Close();
                }
                catch (Exception ex)
                {
                    CustomErrorHandler.GetScript(HttpContext.Current.Response,ex.Message.ToString());
                }
            }
        }
    }
}

我想知道是同时处理Command和DataReader对象，还是它们也会用USING自动处理？

如何改进这个可重用方法类

命令/读取器：they将通过"using"来处理，但仅当您对它们使用"usinng"时，您才应该这样做。

批评：

  • 你把UI和数据访问混合得很糟糕——特别是异常处理没有给调用代码任何指示（尽管我个人也会把控制代码分开），并假设调用方总是想要基于脚本的方法（对我来说，如果这个代码失败了，那就大错特错了：让异常向上冒泡！）
  • 没有适当参数的机制；因此，我的怀疑是，您正在连接字符串，以产生SQL注入的潜在（但非常真实）查询风险
  • 你提到高并发；如果是这样的话，我希望在这里看到一些缓存参与
  • 出于代码维护的原因，我会将所有"创建连接"代码移到一个中心点——"DRY"等；我不希望像这样的单个方法关心连接字符串的来源等细节

坦率地说，我只想在这里使用dapper，并避免所有这些问题：

using(var connection = Config.OpenConnection()) {
     return connection.Query<string>(tsql, args).ToString();
}

（并让调用者在列表上迭代，或者使用AddRange或数据绑定，不管怎样）

总体上同意Marc的回答，但我有一些其他意见和不同的角度。希望我的回答对你有用。

首先，只要不需要任何状态信息，也不共享任何数据，在并发环境中使用静态类和方法就没有错。在你的情况下，填充DropDownList是非常好的，因为你只需要一个字符串列表，一旦完成，你就可以忘记你是如何得到它的。如果对静态方法的并发调用不访问任何静态字段，那么它们之间也不会有干扰。静态方法在.NET框架中很常见，而且它们是线程安全的。

在下面的例子中，我确实使用了一个静态字段log4net记录器。它仍然是线程安全的，因为它不携带任何状态，并且只是log4net库的跳转点，而log4net本身就是线程安全的。建议您至少看看log4net-很棒的日志库。

如果您试图从两个线程中填充相同的下拉列表，这可能是不安全的，但如果这个类不是静态的，这也会是不安全。确保下拉列表是从一个（主）线程填充的。

回到你的代码混合UI和数据检索不是一个好的做法，因为这会降低代码的可维护性和稳定性。把这两个分开。Dapper库可能是简化事物的好方法。我自己没有用过，所以我只能说它看起来非常方便和高效。如果你想/需要学习东西是如何工作的，不要使用它。至少一开始不是这样。

在一个字符串中包含非参数化查询可能会受到SQL注入攻击，但如果该查询不是基于任何直接用户输入构建的，则应该是安全的。当然，你总是可以采用参数化来确定。

使用处理异常

CustomErrorHandler.GetScript(HttpContext.Current.Response, ex.Message.ToString());

对于这个地方来说，感觉太古怪太复杂了，可能会导致另一个例外。处理另一个异常时出现异常意味着恐慌。我会把代码移到外面。如果您在这里需要一些东西，让它成为一个简单的log4net错误日志，并重新抛出该异常。

如果只进行一次DB读取，则不需要显式事务根据连接对象，它在任何情况下都不应该是静态的，而是根据需要创建的这不会对性能造成影响，因为.NET保留了一个现成的连接池，并回收那些"已处理"的连接。

我相信一个例子总是比解释更好，所以下面是我将如何重新安排您的代码。

public static class reuse
{
    static public readonly log4net.ILog log = log4net.LogManager.GetLogger("GeneralLog");
    public static void FillDropDownList(string query, string[] parms,  DropDownList dropDown)
    {
        dropDown.Items.Clear();
        dropDown.DataSource = GetData(query, parms);
        dropDown.DataBind();
    }
    private static IEnumerable<string> GetData(string query, string[] parms)
    {
        using (SqlConnection con = new SqlConnection(GetConnString()))
        {
            try
            {
                List<string> result = new List<string>();
                SqlCommand cmd = new SqlCommand(query, con);
                cmd.Parameters.AddRange(parms);
                SqlDataReader dr = cmd.ExecuteReader();
                if (dr.VisibleFieldCount > 0)
                {
                    while (dr.Read())
                        result.Add(dr[0].ToString());
                }
                dr.Close();
                return result;
            }
            catch (Exception ex)
            {
                log.Error("Exception in GetData()", ex);
                throw;
            }
        }
    }
    private static string GetConnString()
    {
        return ConfigurationManager.ConnectionStrings["MyDbConnection"].ConnectionString.ToString(CultureInfo.InvariantCulture);
    }
}
相关文章：