3

当方法具有多个退出点时,我试图找出跟踪方法返回值的最佳方法。我有几十种方法要添加跟踪。我会通过我尝试过的。

首先,我尝试将每个方法重构为具有这样的单个退出点:

StatusCode CreateResource(string name, string type)
{
    Trace.LogEvent("BEGIN CreateResource name=" + name + " type=" + type);
    StatusCode status = StatusCode.Ok;
    if (!IsValidResourceName(name))
        status = StatusCode.InvalidName;
    else
    {
        if (!IsValidResourceType(type))
            status = StatusCode.InvalidType;
        else
        {
            if (!SystemOnline())
                status = StatusCode.SystemOffline;
            //continues to nest with more conditions
        }
    }
    Trace.LogEvent("END CreateResource result=" + status);
    return status;
}

但是嵌套的 if 语句使它变得丑陋且可读性降低。我大量使用早期退出点作为保护语句,重构它只会造成一团糟,感觉就像解重构。

我尝试的另一件事是将每个方法包装在另一个跟踪返回值的方法中:

StatusCode CreateResource(string name, string type)
{
    Trace.LogEvent("BEGIN CreateResource name=" + name + " type=" + type);
    StatusCode status = CreateResource_DONT_CALL_THIS_METHOD(name, type);
    Trace.LogEvent("END CreateResource result=" + status);
    return status;
}

StatusCode CreateResource_DONT_CALL_THIS_METHOD(string name, string type)
{
    if (!IsValidResourceName(name))
        return StatusCode.InvalidName;
    if (!IsValidResourceType(type))
        return StatusCode.InvalidType;
    if (!SystemOnline())
        return StatusCode.SystemOffline;
    return StatusCode.Ok;
}

问题是如何防止将来其他开发人员(或我)调用包装方法并绕过跟踪,因此包装方法的名称荒谬(且矛盾)。我可以为内部方法定义和调用匿名方法,但这是一个在整个项目中使用的非常混乱的模式。

我发现的最可靠、最易读的方法是这样的,这在 IMO 中还可以,但我怀疑这种 for 语句的使用是否会在代码审查中发挥作用:

StatusCode CreateResource_Internal(string name, string type)
{
    Trace.LogEvent("BEGIN CreateResource name=" + name + " type=" + type);
    StatusCode status = StatusCode.Ok;
    for (int i = 0; i < 1; i++)
    {
        if (!IsValidResourceName(name))
        {
            status = StatusCode.InvalidName;
            break;
        }
        if (!IsValidResourceType(type))
        {
            status = StatusCode.InvalidType;
            break;
        }
        if (!SystemOnline())
        {
            status = StatusCode.SystemOffline;
            break;
        }
    }
    Trace.LogEvent("END CreateResource result=" + status);
    return status;
}

我使用 try finally 块尝试了一个变体:

StatusCode CreateResource_Internal(string name, string type)
{
    Trace.LogEvent("BEGIN CreateResource name=" + name + " type=" + type);
    StatusCode status = StatusCode.Ok;
    try
    {
        if (!IsValidResourceName(name))
        {
            status = StatusCode.InvalidName;
            return status;
        }
        if (!IsValidResourceType(type))
        {
            status = StatusCode.InvalidType;
            return status;
        }
        if (!SystemOnline())
        {
            status = StatusCode.SystemOffline;
            return status;
        }
    }
    finally
    {
        Trace.LogEvent("END CreateResource result=" + status);
    }
    return StatusCode.Ok;
}

这里的缺点是在返回之前可能会忘记设置“状态”变量,在这种情况下将不会执行跟踪。

我的问题是,是否有这样做的最佳实践?我是否应该重构为单个退出点?是否有某种方法可以防止从其他地方调用包装的方法?允许绕过跟踪的危险是否比重构到单个出口点的不整洁更糟糕?

ps 我不想引入像面向方面编程这样繁重的东西。

4

4 回答 4

4

作为一个侧面答案,您可以查看实施,您可以在此处Log4PostSharp的链接中查看教程。它可能不会直接回答您的问题,但会对您的方案有所帮助。

于 2013-08-15T10:25:37.180 回答
4

编写跟踪文件(或任何形式的日志记录)是一个典型的横切关注点示例,您应该尽量避免在您的方法中使用这种类型的代码,因为它会使它们 A) 可读性降低和 B) 重复很多的代码。

我知道您在问题中提到您不想向您的应用程序添加任何 AOP 风格的编程,但我真的建议为此实施 Microsoft Unity。它支持拦截,这正是您在此处尝试解决的场景。只要您遵循良好的接口编码编程实践,您就会惊讶于它是多么容易实现(并且做一些非常酷的事情!)。

只是一些思考的食物......

于 2013-08-15T12:28:54.603 回答
2

我一直使用该try-finally方案,但整个方法都包含在try-block 中。

像这样的东西:

StatusCode CreateResource_Internal(string name, string type)
{
    try
    {
        Trace.LogEvent("BEGIN CreateResource name=" + name + " type=" + type);
        StatusCode status = StatusCode.Ok;

        if (!IsValidResourceName(name))
        {
            status = StatusCode.InvalidName;
            return status;
        }
        if (!IsValidResourceType(type))
        {
            status = StatusCode.InvalidType;
            return status;
        }
        if (!SystemOnline())
        {
            status = StatusCode.SystemOffline;
            return status;
        }
        status = StatusCode.Ok; // A bit silly, but that avoids the problem of status not being set.
        return status;
    }
    finally
    {
        Trace.LogEvent("END CreateResource result=" + status);
    }
}
于 2013-08-15T10:47:39.200 回答
1

我会使用依赖注入——如果每个类都实现了一个接口,那么装饰器模式是最好的解决方案(只是一个代码草图):

interface A
{
  int method1(float x);
}

class AImpl : A
{
    public int method1(float x) { }
}

class LoggedAImpl : A
{
private AImpl innerA;

public int method1(float x) 
{
  //log method and parameters
  int result;
  try
  {
    result = innerA.method1(x);
  }
  finally
  {
    //log method exit
  }
}
}

然后,在应用程序设置中:

new LoggedAImpl(new AImpl()); //pass it everywhere A is needed

将其与统一一起使用似乎很优雅且相对轻松。

如果你不想使用统一,那么至少工厂模式以及上面的就足够了。

于 2013-08-15T10:59:24.237 回答