15

我很高兴编写了一个运行良好且在运行时不会出现任何异常的项目。所以我决定运行静态代码分析工具(我使用的是 Visual Studio 2010)。结果发现违反了规则CA2000,消息如下:

警告 - CA2000:Microsoft.Reliability:在方法“Bar.getDefaultFoo()”中,在对对象“new Foo()”的所有引用超出范围之前调用 System.IDisposable.Dispose。

引用的代码是这样的:

private static IFoo getDefaultFoo()
{
    return (Baz.canIDoIt()) ? new Foo() : null;
}

我自己想:也许条件表达式破坏了逻辑(我的或验证者的)。改为:

private static IFoo getDefaultFoo()
{
    IFoo ret = null;
    if (Baz.canIDoIt())
    {
        retFoo = new Foo();
    }
    return ret;
}

同样的事情又发生了,但现在该对象被称为retFoo. 我用谷歌搜索,我已经 msdn'ed,我已经 stackoverflow'ed。找到这篇文章。创建对象后,我不需要执行任何操作。我只需要返回对它的引用。但是,我尝试应用 OpenPort2 示例中建议的模式。现在代码如下所示:

private static IFoo getDefaultFoo()
{
    Foo tempFoo = null;
    Foo retFoo = null;
    try
    {
        if (Baz.canIDoIt())
        {
            tempFoo = new Foo();
        }
        retFoo= tempFoo;
        tempFoo = null;
    }
    finally
    {
        if (tempFoo != null)
        {
            tempFoo.Dispose();
        }
    }
    return retFoo;
}

再次出现相同的消息,但tempFoo这次变量违反了规则。所以基本上,代码变得扭曲,更长,有点不合理,不必要的复杂,并且做同样的事情,但速度更慢。

我也发现了这个问题,其中相同的规则似乎以类似的方式攻击有效代码。并且建议提问者忽略警告。我也读过这个线程和大量类似的问题。

有什么我错过的吗?规则是否被窃听/无关?我该怎么办?忽视?以某种神奇的方式处理?也许应用一些设计模式?

编辑:

在 Nicole 的请求之后,我以我也尝试使用的形式提交了整个相关代码。

public class DisposableFooTest
{
    public interface IFoo
    {
        void bar();
    }

    public class Foo : IFoo, IDisposable
    {
        public void bar()
        {
            Console.Out.WriteLine("Foo baring now");
        }

        public void Dispose()
        {
            // actual Dispose implementation is irrelevant, or maybe it is?
            // anyway I followed microsoft dispose pattern
            // with Dispose(bool disposing)
        }
    }

    public static class Baz
    {
        private static bool toggle = false;
        public static bool canIDoIt()
        {
            toggle ^= true;
            return toggle;
        }
    }

    private static IFoo getDefaultFoo()
    {
        IFoo result = null;
        try
        {
            if (Baz.canIDoIt())
            {
                result = new Foo();
            }

            return result;
        }
        catch
        {
            if (result != null)
            {
                (result as IDisposable).Dispose();
                // IFoo does not inherit from IDisposable, hence the cast
            }

            throw;
        }
    }

    public static void Main()
    {
        IFoo bar = getDefaultFoo();
    }
}

分析报告包含以下内容:
`CA2000:Microsoft.Reliability:在方法“DisposableFooTest.getDefaultFoo()”中,在对对象“result”的所有引用超出范围之前调用 System.IDisposable.Dispose。%%projectpath%%\DisposableFooTest.cs 44 测试

编辑2:

以下方法解决了 CA2000 问题:

private static IFoo getDefaultFoo()
{
    Foo result = null;
    try
    {
        if (Baz.canIDoIt())
        {
            result = new Foo();
        }
        return result;
    }
    finally
    {
        if (result != null)
        {
            result.Dispose();
        }
    }
}

不幸的是,我不能走那条路。更重要的是,我更希望遵循面向对象的原则、良好实践和指南来简化代码,使其具有可读性、可维护性和可扩展性。我怀疑有人会按预期阅读它:如果可能,给 Foo ,否则为 null 。

4

4 回答 4

10

这是一个误报警告。如果没有代码分析工具警告您没有正确处理它,则无法返回IFoo, if IFooimplements的适当实例。IDisposable

代码分析不会分析您的意图或逻辑,它只是尝试警告常见错误。在这种情况下,“看起来”你正在使用一个IDisposable对象而不是调用Dispose(). 在这里,您是按设计进行的,因为您希望您的方法返回一个新实例,并将其作为工厂方法的一种形式。

于 2013-01-24T00:42:48.053 回答
8

这里的问题不是您的 C# 代码明确执行的操作,而是生成的 IL 使用多个指令来完成 C# 代码中的一个“步骤”。

例如,在第三个版本中(使用 try/finally),该return retFoo行实际上涉及分配由编译器生成的附加变量,而您在原始代码中看不到该变量。由于此分配发生在 try/finally 块之外,它确实会引入未处理异常的可能性,从而使您的一次性 Foo 实例“孤立”。

这是一个可以避免潜在问题(并通过 CA2000 筛选)的版本:

private static IFoo getDefaultFoo()
{
    IFoo result = null;
    try
    {
        if (Baz.canIDoIt())
        {
            result = new Foo();
        }

        return result;
    }
    catch
    {
        if (result != null)
        {
            result.Dispose();
        }

        throw;
    }
}

显然,与第一个版本相比,这严重影响了可读性和可维护性,因此您必须决定孤立的可能性和不处理孤立的后果是否真的值得更改代码,或者是否抑制 CA2000 违规在这种特殊情况下可能更可取。

于 2013-01-24T13:40:06.933 回答
2

静态分析基本上是在抱怨,原因如下:

  1. IFoo接口不继承自IDisposable,
  2. 但是您正在返回一个必须处理的具体实现,而调用者将无法知道这一点。

换句话说,您的GetDefaultFoo方法的调用者需要一个IFoo实现,但不知道您的实现需要显式处置,因此可能不会处置它。如果这是其他库中的常见做法,您将不得不手动检查项目是否IDisposable实现了任何可能接口的每个可能实现。

显然,这是有问题的。

恕我直言,解决此问题的最简洁方法是IFoo继承自IDisposable

public interface IFoo : IDisposable
{
    void Bar();
}

这样,调用者就知道他们收到的任何可能的实现都必须被处理掉。这也将允许静态分析器检查您使用IFoo对象的所有位置,并在您没有​​正确处理它们时警告您。

于 2013-01-25T13:20:16.667 回答
-1

返回 IDisposable 对象时,如果值为 null,请处置该对象并在方法中返回 null - 这应该清除与您的“私有静态 IFoo getDefaultFoo()”方法关联的“警告/建议”消息。当然,您仍然需要在调用例程中将对象作为 IDisposable(使用或处置)来处理。

private static IFoo getDefaultFoo()
{
    IFoo ret = null;
    if (Baz.canIDoIt())
    {
        retFoo = new Foo();
    }
    return ret;
}

改成

private static IFoo getDefaultFoo()
{
    IFoo ret = null;
    if (Baz.canIDoIt())
    {
        retFoo = new Foo();
    }
    if (ret == null)
    {
        ret.dispose();
        return null;
    }
    else
        return ret;
}
于 2019-11-29T06:34:31.760 回答