如何避免代码中自定义日志消息的复杂逻辑



我知道标题有点过于宽泛,但我想知道如何避免(如果可能的话)我刚刚在我们的解决方案中编写的这段代码。

当此代码导致没有足够的日志信息时,问题开始了:

...
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;
...

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

对于这个看似简单的问题,还有其他方法吗?这似乎是一种违反单一责任原则的行为,登录在代码和所有这些之间,但我看不到一个体面或优雅的解决方法。在我们的情况下,情况更糟,因为相同的步骤重复了两次,一次是对"用户",然后是对"设备",如下所示:

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

对于这两种操作,重要的是我们要确切地知道发生了什么,在不是唯一的情况下返回了什么用户/设备,诸如此类的事情。

@AntP找到了我最喜欢的答案。我认为你之所以挣扎是因为你实际上有两个问题。首先,代码似乎承担了太多的责任。应用这个简单的测试:给这个方法一个简单的名称,描述它所做的一切。如果你的名字里有"one_answers"这个词,那就太过分了。当我应用该测试时,我可能会将其命名为"GetUsersByWriter AndValidateOnlyOneUserMatches()"。所以它做了两件事。将其拆分为一个查找函数和一个单独的函数,前者不关心返回了多少用户,后者评估关于"我只能处理一个返回的用户"的业务规则。

不过,你仍然有最初的问题,这就是switch语句在这里看起来很尴尬。当查看switch语句时,会想到策略模式,尽管在这种情况下,我会认为它过于夸张。

不过,如果您想探索它,可以考虑创建一个基本的"UserSearchResponseHandler"类和三个子类:NoUsersReturn;多用户返回;和OneUserReturned。它将有一个工厂方法,它将接受一个用户列表,并根据用户数返回一个UserSearchResponseHandler(将开关的逻辑封装在工厂内)。每个处理程序方法都会做正确的事情:记录适当的内容,然后返回null,或返回一个用户。

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

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

另一种不同的考虑方式是进行一些金发姑娘测试。如果这真的是一个错误条件,你甚至可以抛出:

if (users.count() < 1)
{
    throw TooFewUsersReturnedError;
}
if (users.count() > 1)
{
    throw TooManyUsersReturnedError;
}
return users[0];  // just right

这样的东西怎么样?

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方法更像是一个返回有关结果的一些数据和元数据的服务,它允许您将日志记录委托给它所属的调用者,这样您的数据访问代码就可以继续执行它的工作。

不过,老实说,在这种情况下,我更愿意简单地从GetMappedUsers返回一个用户ID列表,然后使用users.Count在调用者中评估您的"案例",并根据需要进行日志记录。

最新更新