IDisposable, созданный в рамках метода и возвращенный
Я счастлив, закодировал довольно проект, который отлично работает и не проявляет каких-либо странностей во время выполнения. Поэтому я решил запустить статический инструмент анализа кода (я использую Visual Studio 2010). Выяснилось, что правило CA2000
нарушается, сообщение выглядит следующим образом:
Предупреждение - CA2000: Microsoft.Reliability: В методе "Bar.getDefaultFoo()" вызовите System.IDisposable. Разместите объект "new Foo()" перед тем, как все ссылки на него выходят за рамки.
Указанный код выглядит следующим образом:
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
. Я googled, я 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
является нарушителем правил на этот раз. Так что, в основном, код стал скрученным, дольше, немного иррациональным, неуклюже сложным и делает то же самое, но медленнее.
Я также нашел этот вопрос, где такое же правило, похоже, атакует действительный код аналогичным образом. И есть вопрос, чтобы игнорировать предупреждение. Я также прочитал эту тему и массу подобных вопросов.
Я что-то пропустил? Является ли правило прослушиванием/неуместным? Что мне делать? Игнорировать? Рукоятка каким-то магическим способом? Может быть, применить шаблон дизайна?
Edit:
После запроса 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()' вызывать System.IDisposable.Dispose на объекте 'result', прежде чем все ссылки на него выходят за рамки. %% projectpath %%\DisposableFooTest.cs 44 Тест
Edit2:
Следующий подход разрешил проблему 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 в противном случае.
Ответы
Ответ 1
Это ложное положительное предупреждение. Невозможно вернуть соответствующий экземпляр IFoo
, если IFoo
реализует IDisposable
, без инструмента анализа кода, предупреждающего вас о том, что вы не распоряжаетесь им должным образом.
Анализ кода не анализирует ваши намерения или логику, он просто пытается предупредить об общих ошибках. В этом случае он выглядит так: вы используете объект IDisposable
и не вызываете Dispose()
. Здесь вы делаете это по дизайну, так как хотите, чтобы ваш метод возвращал новый экземпляр и действовал как форма метода factory.
Ответ 2
Проблема здесь не в том, что ваш код С# явно делает, а скорее, что сгенерированный IL использует несколько инструкций для выполнения того, что выглядит как "шаг" в коде С#.
Например, в третьей версии (с попыткой/окончательно) строка 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 может быть предпочтительным в данном конкретном случае.
Ответ 3
Статический анализ в основном жалуется по следующей причине:
- Интерфейс
IFoo
не наследует от IDisposable
,
- Но вы возвращаете конкретную реализацию, которая должна быть утилизирована, и у абонентов не будет способа узнать это.
Другими словами, вызывающий объект вашего метода GetDefaultFoo
ожидает реализацию IFoo
, но не имеет понятия, что ваша реализация должна быть явно удалена, и поэтому, вероятно, не будет ее уничтожать. Если это была обычная практика в других библиотеках, вам нужно будет вручную проверить, реализует ли элемент IDisposable
для, ну, всякую возможную реализацию любого возможного интерфейса.
Очевидно, что с этим что-то не так.
IMHO, самый чистый способ исправить это - сделать IFoo
наследовать от IDisposable
:
public interface IFoo : IDisposable
{
void Bar();
}
Таким образом, абоненты знают, что любая возможная реализация, которую они получают, должна быть удалена. И это также позволит статическому анализатору проверять все места, где вы используете объект IFoo
, и предупреждать вас, если вы не используете их правильно.