Pull to refresh

Comments 32

Вообще-то, свойства не должны вызывать исключения.
И если они их могут создать — заменяется на функцию.

Сейчас лень искать пруф на guidelines.
Здесь есть пара «но». Во-первых, я привел пример реального свойста, которое бросает исключение (на то они и сеттеры, чтобы бросать исключения), во-вторых, исключение может бросать не само свойство, а выражение получение значения:

using (var reader = ...) // Получаем DataReader
using (var person = new Person 
  {
     Id = (int)reader["id"], 
     Name = (string)reader["name"], // падает индексатор
  })
{
}


Поведение будет аналогичным.
[сарказм] Какие же программисты в Майкрософт плохие, что нарушает Ваш гидлайн! [/сарказм] См., например, описание к свойству Position
Да, согласен, лучше избегать исключений в свойствах, но, согласитесь, без исключений не всегда можно обойтись.
Я же уже сказала: заменяйте на методы.
В данном случаи, упрощение в инициализации объекта по средством публичных сеттеров — не самая лучшая идея.

Майкрософт — не идеальны, но в ваших проектах, — старайтесь избегать подобных решений, когда свойства могут генерировать исключения. Заменяйте их на фукнции.
А я таки не поленился достать с полки Framework Design Guidelines.

Раздел 5.2 Property Design:

DO preserver the previous value if a property setter throws an exception.
AVOID throwing exceptions from property getters.

Так что никаких проблем с сеттерами, бросающими исключения нет. Да и не о том же речь вообще; дело же не в том, как вы будете проектировать свои типы, дело же в том, что в очень простом коде может крыться ошибка, а проявляться она может, как в случае со свойством, так в случае и с любым другим методом, генерирующим исключение (см. мой второй пример в этом комментарии).
«Methods that are not expected to throw exceptions can be categorized as follows:

Property Get Methods

Event Accessor Methods...» Смотреть здесь. Не вижу, чтобы там было что-то сказано о setter

Помоему, самым разумным тут будет изменить поведение компилятора, чтоб он учитывал такую возможность.

long position = -1;
var tmpFile = new FileStream(«d:\\1.txt», FileMode.OpenOrCreate);
try
{
tmpFile.Position = position;
var file = tmpFile;

}
finally
{
if (file != null)
((IDisposable)file).Dispose();
}
Почему бы самому тогда не написать присвоение Position внутри using? Кода становится больше максимум на пару-тройку строк, а скорее всего останется столько же. По-моему, этот инициализатор объекта, да ещё и вместе using, да ещё если и не один объект создаётся составляют нечитаемую конструкцию.
Есть сомнение в том, что поведение будет изменено. Вот ответ Эрика Липперта по этому поводу:

The behaviour is correct and desirable. The spec is extremely clear on this point and if an implementation of C# does not have this behaviour, they have a bug.

The spec clearly states that «using(T x = y) s;» means exactly the same thing as "{ T x = y; try { s; } finally {… code to dispose x… }". That is, the initialization is done outside the try-protected region, precisely so that if it fails, we do NOT attempt to clean it up. That would be dangerous.

The spec also clearly states that «t = new T() { X = x };» means exactly the same thing as «T temp = new T(); temp.X = x; t = temp;». That is, the assignment to the variable happens after the object is known to be in its fully initialized state.

These two designs work together to ensure that you never dispose a partially initialized resource. If you have a situation where you need to handle disposing of partially constructed resources then you need to write the code yourself to do that; there's no way we can generate the code for you and have any confidence that it is right. Any operation on a partially initialized resource is potentially hazardous.
Интересно. И, имхо, не правильно.
У конструкции using основная задача — вызвать Dispose() у класса, который имеет дело с неуправляемыми ресурсами. Единственным условием для вызова Dispose() является отработка конструктора без ошибок (со своими ошибками он либо должен справляться сам внутри, либо не выкидывать их, ещё до того, как мы вошли в try блок, в который развернулся using).
Но вот если конструктор отработал, то Dispose должен быть вызван — ради этого вся конструкция using и делалась. Таким образом, имхо, у try{} блока в данном случае есть только одно логическое место начала — сразу за конструктором.

Более того, именно такого поведения я от него и жду, это надёжно.
Я понимаю, что сахарок удобен, но что бы он был реально удобен, нужно, чтоб он был ещё и надежен. Да можно отписаться, что у нас есть две сладкие конструкции, которые так и так разворачиваются, но их совместное использование разрушает принцип надежности программы. Сахар на то и сахар, чтоб программист не особо заморачивался с тем что там внутри. Если создали два куска сахара — добавьте третий для их совместного использования. Во первых это не так сложно в данной ситуации, во вторых сахар это все-таки не макро определение, а часть языка и программиста не должно особо волновать, что там внутри. Напротив, компилятор должен продумать возможные проблемы, как он это делает с другими встроенными конструкциями. Или запретить использование этой конструкции в данном месте.

Вот пример элементарного разворота этой конструкции во вполне надежный код:
long position = -1;
var tmpFile = new FileStream("d:\\1.txt", FileMode.OpenOrCreate);
 
try
{
    tmpFile.Position = position;
    var file = tmpFile;
    //....
}
finally
{
    if (tmpFile != null)
        ((IDisposable)tmpFile).Dispose();
}

При этом переменная file должна определяться всегда внутри try, а конструкция using должна всегда использовать сгенерированную компилятором переменную для вызова Dispose. Тогда все выглядит вполне сладко и надежно. Поправьте, если я ошибаюсь.
Мне кажется, что проблемы не существует при понимании работы using(...). Все что в скобках — не финализируется; в теле — финализируется.

А вообще напоминает объявление переменных через запятую: int a,b;
Я когда это вижу всегда коллег спрашиваю: «Что это?». Когда получаю ответ — до меня доходит. Вот и здесь сидел и думал: «А в чем суть?». Смешно то, что +2 строки. Удачный сахарок. Я не про инициализацию вообще, только вместе с using. Не конструкция — беременный гиппопотам.

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

using(new A("a")
	{
		B=1
	})
{
	//какой-то код
}


точно не лучше

using(var a=new A("a"))
{
	a.B=1;
	//какой-то код
}

В любом случае тогда должно быть понимание того, что исключение в конструкторе не ловится конструкцией using.
B = 1, конечно, разворачивается не в конструктор, но синтаксически выглядит как «конструктор» (по сути «замена» конструктору, т.е. как и предполагалось), поэтому я считаю, что поведение using здесь правильное и привычное.
полезный пост (правда где то я его уже читал).
буквально на днях столкнулся с аналогичной ситауцией, когда объект создается методом, которые много чего делает, в том числе может выкинуть исключение. вот сейчас думаю, как этот код переписать красиво.

using (IFile file = new CreateFile())
{
//…
}
IFile CreateFile()
{
IFile file = new File();
//… код, который может вызывать Exception
return file;
}
Это таже ситуация, что и с конструктором. Что делать, если в конструкторе вначале захватывается ресурс, а потом может быть сгенерировано исключение? Тоже самое, обрамлять в try/catch, и в блоке catch освобождать ресурс и пробрасывать исключение.

IFile CreateFile()
{
  var file = new File();

  try
  {
    //… код, который может вызывать Exception
    return file;
  }
  catch(Exception)
  {
    file.Close();
    throw;
  }
}


В С++ можно было бы надеяться на автоматическое удаление при выходе из области видимости (a.k.a. RAII), но в C# только рукопашная.
кроме рукопашной я еще использую финалайзер: иногда помогает.

Все-таки между конструктором и внешней инициализацией объекта есть существенная разница: конструктор легче написать правильно и с помощью код-ревью увидеть проблему.
почему у меня код так красиво не показывается?
вроде теги source & code использую.

IFile CreateFile()
{
}

надо выделить код и в комбобоксе выбрать C#
так я так и делаю (вот пример, только квадратные скобки вместо треугольных)

[source lang=«cs»]
IFile CreateFile()
{
}
[/source]

IFile CreateFile()
{
}

UFO just landed and posted this here
Объект FileStream будет создан, лишь переменная не будет проинициализирована. А это значит, что детерминированного (a.k.a. по выходу из области видимости) освобождения мы не получим.
UFO just landed and posted this here
Как это не будут? Объект будет обязательно создан, только ссылка потеряется. И при попытке, удалить файл до сборки мусора поднимется исключение по доступу.

Выполнение описываемых предписаний не защитят вас от проблем между строками «var tmpFile» и «try» (например, из-за асинхронных исключений). Они скорее говорят как уменьшать вероятность возникновения проблем, уменьшив прослойку. Для полной уверенности надо предпринимать отдельные усилия, подобные habrahabr.ru/post/144240/#comment_4841489
Объект будет создан, и даже соберется в свой час мусорщиком, однако не будет вызова Dispose. Обычно это не страшно, но представьте себе ситуацию, когда в диспозере вызывается, например, не откат транзакции, а ее коммит (я понимаю, что пример так себе, но представим, что у нас программист-параноик, который журналирует ошибки конструкторов в транзакционное хранилище). Так вот в нашем случае записи в журнал не будет. Поэтому, как правильно сказал коллега выше по тексту, не усердствуйте с использованием инициализаторов там, где этого требуется, а если выхода нет, то отказывайтесь от сахара using в пользу специфической обработки исключений.
Sign up to leave a comment.

Articles