Утилизировать или не утилизировать (CA2000)


Я включаю анализ кода в более старом проекте. Большинство замечаний, которые приводят к этому, я могу понять, но CA2000: Dispose objects before losing scope трудно получить правильно.

Например, этот код из ASP.Net Страница:

private void BuildTable()
{
    HtmlTableRow tr = new HtmlTableRow();
    HtmlTableCell td = new HtmlTableCell();

    tr.Cells.Add(td);
    // add some controls to 'td'

    theTable.Rows.Insert(0, tr);
    // 'theTable' is an HtmlTable control on the page
}

Дает сообщения CA:

CA2000: Microsoft.Надежность: в методе ' BuildTable ()', вызовите систему.Интерфейс IDisposable.Размещайте на объекте 'tr' до того, как все ссылки на него выйдут за пределы области видимости.

CA2000 : Microsoft.Надежность: в методе 'BuildTable ()' объект ' td ' не расположен вдоль всех путей исключения. Система Вызова.Интерфейс IDisposable.Размещайте на объекте 'td' до того, как все ссылки на него выйдут за пределы области видимости. (и аналогичные сообщения об элементах управления, которые добавляются к этому "td".)

Я могу решить вторую проблему:

private void BuildTable()
{
    HtmlTableRow tr = new HtmlTableRow();
    HtmlTableCell td = new HtmlTableCell();

    try
    {
        tr.Cells.Add(td);
        // add some controls to 'td'

        td = null; // this line is only reached when there were no exceptions
    }
    finally
    {
        // only dispose if there were problems ('exception path')
        if (td != null) td.Dispose();
    }

    theTable.Rows.Insert(0, tr);
}

Но я не думаю, что можно разрешить сообщение о 'tr'. Я не могу избавиться от этого, потому что он все еще нужен после того, как метод был применен. возбужденный.

Или я что-то пропустил?

Кстати: изменение этого theTable.Rows.Insert в theTable.Rows.Add изменяет сообщение CA на 'не расположенное вдоль всехисключительных путей'

5 13

5 ответов:

Анализ кода не может полностью понять ваш код и просто предупреждает, если вы создаете одноразовый объект, который, кажется, не утилизируется. В вашем случае вы должны отключить предупреждение, потому что объект не должен быть удален до выхода из метода. Вы можете отключить предупреждения либо для всего проекта, настроив набор правил анализа кода, либо для каждого метода, имеющего это предупреждение, когда очевидно, что анализ кода неверен.

Тем не менее, я рекомендую вам используйте конструкцию using при работе с объектами IDisposable:

using (var tr = new HtmlTableRow()) {
  using (var td = new HtmlTableCell()) {
    tr.Cells.Add(td);
    theTable.Rows.Insert(0, tr);
  }
}

За исключением того, что этот код является бессмыслицей, поскольку вы не хотите избавляться от строки и ячейки, которые вы только что добавили в таблицу.

Я думаю, что вы только что показали, что правило CA2000 не очень полезно на большинстве кодовых баз Насколько мне известно,

  • Dispose on HtmlTableRow не делает ничего полезного, если он не используется внутри конструктора пользовательского интерфейса; я никогда не видел, чтобы кто-то вызывал dispose на Asp.net управление. (Winforms/WPF-это другой случай)
  • вы храните ссылку на td внутри таблицы, поэтому вы не должны выбрасывать ее в любом случае.

Поскольку оба вышеперечисленных метода очень распространены в нормальном коде, я не смотрите, что правило CA2000 имеет значение для большинства кодовых баз – естьтак много ложных срабатываний , которые вы, скорее всего, пропустите в 1 из 50 случаев, когда это реальная проблема.

Этот код избавит вас от обоих предупреждений. (Я использую using (HtmlTable) для имитации вашего глобального члена HtmlTable...):

using (HtmlTable theTable = new HtmlTable())
{
    HtmlTableRow tr = null;
    try
    {
        HtmlTableCell td = null;

        try
        {
            td = new HtmlTableCell();

            // add some controls to 'td'


            tr = new HtmlTableRow();
            tr.Cells.Add(td);

            /* td will now be disposed by tr.Dispose() */
            td = null;
        }
        finally
        {
            if (td != null)
            {
                td.Dispose();
                td = null;
            }
        }

        theTable.Rows.Insert(0, tr);

        /* tr will now be disposed by theTable.Dispose() */
        tr = null;
    }
    finally
    {
        if (tr != null)
        {
            tr.Dispose();
            tr = null;
        }
    }
}
Но я думаю, что вы рассмотрите возможность использования подхода, который использует подфункции, чтобы сделать код более ясным:
    private static void createTable()
    {
        using (HtmlTable theTable = new HtmlTable())
        {
            createRows(theTable);
        }
    }

    private static void createRows(HtmlTable theTable)
    {
        HtmlTableRow tr = null;
        try
        {
            tr = new HtmlTableRow();
            createCells(tr);

            theTable.Rows.Insert(0, tr);

            /* tr will now be disposed by theTable.Dispose() */
            tr = null;
        }
        finally
        {
            if (tr != null)
            {
                tr.Dispose();
                tr = null;
            }
        }
    }

    private static void createCells(HtmlTableRow tr)
    {
        HtmlTableCell td = null;

        try
        {
            td = new HtmlTableCell();

            // add some controls to 'td'


            tr.Cells.Add(td);

            /* td will now be disposed by tr.Dispose() */
            td = null;
        }
        finally
        {
            if (td != null)
            {
                td.Dispose();
                td = null;
            }
        }
    }

Добавьте элемент управления в коллекцию непосредственно после его создания, но до того, как вы что-либо сделаете с элементом управления.

HtmlTableRow tr = new HtmlTableRow();
theTable.Rows.Insert(0, tr);

HtmlTableCell td = new HtmlTableCell();
tr.Cells.Add(td);

// add some controls to 'td'

Поскольку не может быть исключения между созданием и добавлением/вставкой элемента управления в коллекцию, нет необходимости в try / catch. Если после добавления элемента управления в коллекцию возникает исключение, страница его удалит. Вы не получите CA2000 таким образом.

В случае, если вы считаете, что анализ кода ошибочен (случилось со мной, что он попросил вызвать Dispose для объекта, который не реализовал IDisposable), или вы не чувствуете, что необходимо удалить этот объект, вы всегда можете подавить это сообщение следующим образом.

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000: DisposeObjectsBeforeLosingScope")]
public static IDataReader RetrieveData(string conn, string sql)
{
    SqlConnection connection = new SqlConnection(conn);
    SqlCommand command = new SqlCommand(sql, conn);
    return command.ExecuteReader(CommandBehavior.CloseConnection);
    //Oops, I forgot to dispose of the command, and now I don't get warned about that.
}