Как избежать анализа кода CA2000 предупреждений с помощью цепных конструкторов? - PullRequest
4 голосов
/ 19 июля 2011

Обычный способ избежать предупреждения CA2000 о неразмещенных локальных компьютерах - это использовать временную переменную, которая удаляется, если что-то идет не так, как:

Foo f = null;
try
{
    f = new Foo();
    Foo result = f;
    f = null;
    return result;
}
finally
{
    if (f != null)
    {
        f.Dispose();
    }
}

Немного многословно, но это работает, и это имеет смысл. Но как применить этот шаблон к цепочечным конструкторам, например:

public HomeController ( IDataRepository db )
{
    this.repo = db ?? new SqlDataRepository();
}

public HomeController ( )
    : this(new SqlDataRepository())
{
}

Этот код выдает два предупреждения CA2000, по одному на конструктор. Первый, от которого я могу избавиться, используя шаблон временных переменных. Это довольно раздражает, поскольку у локального пользователя нет никакого способа выйти из области видимости после его создания, но до того, как он будет назначен полю-члену, которое будет очищено позже. Так что я не знаю, в чем проблема CA, но, по крайней мере, я знаю, что нужно сделать, чтобы ее исправить.

Но, насколько я могу судить, альтернативного способа написать второй вызов конструктора для введения try / finally нет. Назначаемое поле доступно только для чтения, поэтому в конструкторе должно быть установлено . А C # не позволит вам вызывать цепочечные конструкторы нигде, кроме непосредственно перед телом конструктора. И, из двух, второе предупреждение на самом деле является более законным - вызов цепочечного конструктора может (теоретически, если он выполнил какую-либо реальную работу) вызвать исключение и оставить вновь созданный параметр нераспределенным.

Конечно, я всегда могу просто подавить это сообщение (которое, как кажется, CA2000 очень нужно), но если есть реальный способ устранить проблему, я бы предпочел это сделать.

1 Ответ

2 голосов
/ 19 июля 2011

Я не могу воспроизвести нарушение CA2000 на конструкторе, который принимает аргумент IDataRepository. Учитывая это, а также тот факт, что вы используете одно и то же «значение по умолчанию» для обоих конструкторов, самое простое изменение, которое позволит избежать проблем CA2000 для примера сценария:

public HomeController(IDataRepository db)
{
    this.repo = db ?? new SqlDataRepository();
}

public HomeController()
    : this(null)
{
}

Очевидно, что это не сработало бы слишком хорошо, если бы ваш первый конструктор не принял нулевое значение параметра. Если бы это было так, и вы абсолютно женаты на идее установки соответствующего поля только в одном месте, у вас все равно были бы возможности избежать CA2000, например, вызывая немного более умный частный конструктор. e.g.:

public HomeController(IDataRepository db)
    : this(() => db, false)
{
    if (db == null)
    {
        throw new ArgumentNullException("db");
    }
}

public HomeController()
    : this(() => new SqlDataRepository(), true)
{
}

private HomeController(Func<IDataRepository> repositoryRetriever, bool disposeOnFailure)
{
    IDataRepository repository = repositoryRetriever.Invoke();
    try
    {
        this.repo = repository;
    }
    catch
    {
        if (disposeOnFailure)
        {
            repository.Dispose();
        }

        throw;
    }
}

Лично я считаю, что вышесказанное - довольно неприятный хак, особенно учитывая, что оно включает в себя увеличение как сложности кода, так и шансов исключения во время выполнения в интересах избежания потенциальной проблемы, которая вначале не была очень серьезной. Я бы порекомендовал просто игнорировать потенциальные нарушения CA2000 такого рода, если не выполняются оба следующих условия:

  1. Существует реальная вероятность возникновения неустранимого исключения между созданием объекта и завершением метода, в котором он назначен полю.
  2. Последствия неспособности избавиться от осиротевшего экземпляра довольно серьезны (например, оставить файл заблокированным).
...