0

我知道标题有点过于宽泛，但我想知道如何避免（如果可能）我刚刚在我们的解决方案上编写的这段代码。

当此代码导致日志信息不足时，问题就开始了：

...
var users = [someRemotingProxy].GetUsers([someCriteria]);
try
{
    var user = users.Single();
}
catch (InvalidOperationException)
{
    logger.WarnFormat("Either there are no users corresponding to the search or there are multiple users matching the same criteria.");
    return;
}
...

我们的模块中有一个业务逻辑，需要有一个匹配某些标准的“用户”。事实证明，当问题开始出现时，这个小小的“不确定”信息不足以让我们正确知道发生了什么，所以我编写了这个方法：

private User GetMappedUser([searchCriteria])
{
    var users = [remotingProxy]
           .GetUsers([searchCriteria])
           .ToList();

    switch (users.Count())
    {
        case 0:
            log.Warn("No user exists with [searchCriteria]");
            return null;

        case 1:
            return users.Single();

        default:
            log.WarnFormat("{0} users [{1}] have been found"
                          users.Count(), 
                          String.Join(", ", users);
            return null;
    }

然后像这样从主代码中调用它：

...
var user = GetMappedUser([searchCriteria]);
if (user == null) return;
...

我看到的第一件奇怪的事情是清单上的switch声明。.Count()起初这似乎很奇怪，但不知何故最终成为更清洁的解决方案。我在这里尝试避免异常，因为这些情况很正常，而且我听说尝试使用异常来控制程序流程而不是报告实际错误是不好的。代码之前是从 Single 抛出的InvalidOperationException，所以这更像是一个重构。

对于这个看似简单的问题，还有另一种方法吗？这似乎是一种违反单一责任原则的行为，日志介于代码和所有内容之间，但我看不到一个体面或优雅的方法。在我们的例子中情况更糟，因为相同的步骤重复了两次，一次是针对“用户”，然后是针对“设备”，如下所示：

  1. 获取唯一用户
  2. 获取唯一用户的唯一设备

对于这两种操作，重要的是我们要确切地知道发生了什么，如果它不是唯一的，则返回了哪些用户/设备，诸如此类。

4

2 回答 2

1

@AntP 找到了我最喜欢的答案。我认为你挣扎的原因是你实际上有两个问题。首先是代码似乎有太多的责任。应用这个简单的测试：给这个方法一个简单的名字来描述它所做的一切。如果你的名字中包含“和”这个词，那就太过分了。当我应用该测试时，我可能会将其命名为“GetUsersByCriteriaAndValidateOnlyOneUserMatches()”。所以它正在做两件事。将其拆分为一个不关心返回多少用户的查找函数，以及一个单独的函数，用于评估关于“我只能处理一个返回的用户”的业务规则。

但是，您仍然有最初的问题，那就是 switch 语句在这里看起来很尴尬。在查看 switch 语句时会想到策略模式，尽管在这种情况下我会认为它有点矫枉过正。

但是，如果您想探索它，请考虑创建一个基本“UserSearchResponseHandler”类和三个子类：NoUsersReturned；多个用户返回；和 OneUserReturned。它将有一个工厂方法，该方法将接受用户列表并根据用户数量返回 UserSearchResponseHandler（封装工厂内的开关逻辑。）每个处理程序方法都会做正确的事情：记录适当的内容然后返回 null ，或返回单个用户。

当您对它所识别的数据有多种需求时，策略模式的主要优势就出现了。如果您的代码中隐藏了 switch 语句，这些语句都取决于搜索找到的用户数，那么这将是非常合适的。工厂还可以封装更复杂的规则，例如“user.count must = 1 AND the user[0].level must = 42 AND it must be a Tuesday in September”。您还可以非常喜欢工厂并使用注册表，从而允许对逻辑进行动态更改。最后，工厂很好地将业务规则的“解释”与规则的“处理”分开。

但在你的情况下，可能不是那么多。我猜你可能只有一次出现这个规则，它看起来很静态，并且它已经适当地位于你获取它正在验证的信息的点附近。虽然我仍然建议从响应解析器中分离搜索，但我可能只是使用开关。

考虑它的另一种方法是使用一些 Goldilocks 测试。如果它确实是一个错误条件，你甚至可以抛出：

if (users.count() < 1)
{
    throw TooFewUsersReturnedError;
}

if (users.count() > 1)
{
    throw TooManyUsersReturnedError;
}

return users[0];  // just right
于 2013-09-10T20:04:31.470 回答
0

这样的事情怎么样？

public class UserResult
{
    public string Warning { get; set; }
    public IEnumerable<User> Result { get; set; }
}

public UserResult GetMappedUsers(/* params */) { }

public void Whatever()
{
    var users = GetMappedUsers(/* params */);
    if (!String.IsNullOrEmpty(users.Warning))
        log.Warn(users.Warning);
}

List<string> Warnings如果需要，请切换。这将您的GetMappedUsers方法更像是一种服务，它返回一些数据和一些关于结果的元数据，它允许您将日志记录委托给调用者 - 它所属的位置 - 这样您的数据访问代码就可以继续完成它的工作。

虽然，老实说，在这种情况下，我更愿意简单地返回一个用户 ID 列表，GetMappedUsers然后用于users.Count评估调用者中的“案例”并酌情记录。

于 2013-09-03T19:59:55.963 回答