Всем привет!

Предсказуемы ли результаты выполнения тестов?

class Disposable : IDisposable
{
    public bool Disposed { get; set; }
    public void Dispose()
    {
        Disposed = true;
    }
}

[Test]
public void Test1()
{
    Disposable disposable = null;

    using(disposable)
    {
        disposable = new Disposable();
    }
    Assert.IsTrue(disposable.Disposed);
}

[Test]
public void Test2()
{
    Disposable disposable = null;

    try
    {
        disposable = new Disposable();
    }
    finally
    {
        if(disposable != null)
        {
            disposable.Dispose();
        }
    }
    Assert.IsTrue(disposable.Disposed);
}

А главным образом, что бы Вы мне сказали на месте разработчика Code Analysis тула?

Здравствуйте, samius, Вы писали:

Кстати компилятор C# выдает предупреждение:

warning CS0728: Possibly incorrect assignment to local 'disposable' which is the argument to a using or lock statement.
The Dispose call or unlocking will happen on the original value of the local.

Здравствуйте, _nn_, Вы писали:

__>Здравствуйте, samius, Вы писали:

__>Кстати компилятор C# выдает предупреждение:

__>warning CS0728: Possibly incorrect assignment to local 'disposable' which is the argument to a using or lock statement.
__>The Dispose call or unlocking will happen on the original value of the local.

Воистину так.
Случилось, что обнаружил предупреждение уже после того как...

Текущую версию документации дополнили (using Statement)

Within the using block, the object is read-only and cannot be modified or reassigned.

а в старой ничего такого не было. Собственно эффект стал для меня неожиданностью, т.к. при переходе на vs2008 документацию к ранним фичам не перечитывал, а рассчитывал на известную мне по MSDN схему разворачивания блока using в try/finally, не подозревая, что using образует 2 локальных переменных.

Здравствуйте, samius, Вы писали:

[]

S>Within the using block, the object is read-only and cannot be modified or reassigned. [/q]
S>а в старой ничего такого не было. Собственно эффект стал для меня неожиданностью, т.к. при переходе на vs2008 документацию к ранним фичам не перечитывал, а рассчитывал на известную мне по MSDN схему разворачивания блока using в try/finally, не подозревая, что using образует 2 локальных переменных.

а что там за код теперь?

var ob = new T();
var _ob = ob;
try
{
   ...
}
finally
{
  _ob.Dispose();
}

?

Я для себя сделал правилом — объект в using рождается, объект в using и умирает. Внутри никаких манипуляций с ссылкой.

Здравствуйте, Константин Л., Вы писали:

КЛ>а что там за код теперь?

КЛ>

КЛ>var ob = new T();
КЛ>var _ob = ob;
КЛ>try
КЛ>{
КЛ>   ...
КЛ>}
КЛ>finally
КЛ>{
КЛ>  _ob.Dispose();
КЛ>}
КЛ>

КЛ>?

Да, только с проверкой _ob на null в блоке finally.

КЛ>Я для себя сделал правилом — объект в using рождается, объект в using и умирает. Внутри никаких манипуляций с ссылкой.
Хорошее правило.

Сам раньше при необходимости пользовался следующим:

var d = ...;
using(d)
{
DoSomething();
}

Но сейчас и от этого предостерегают в MSDN (плохо то, что переменная d доступна после блока using, когда уже гарантированно отработал Dispose).

Здравствуйте, samius, Вы писали:

[]

S>Сам раньше при необходимости пользовался следующим:

S>var d = ...;
S>using(d)
S>{
S> DoSomething();
S>}

вот этого, по моему мнению, делать не стоит. опасно

S>Но сейчас и от этого предостерегают в MSDN (плохо то, что переменная d доступна после блока using, когда уже гарантированно отработал Dispose).

давно пора

Здравствуйте, samius, Вы писали:

КЛ>>Я для себя сделал правилом — объект в using рождается, объект в using и умирает. Внутри никаких манипуляций с ссылкой.
S>Хорошее правило.
S>Сам раньше при необходимости пользовался следующим:

S>var d = ...;
S>using(d)
S>{
S>    DoSomething();
S>}

S>Но сейчас и от этого предостерегают в MSDN (плохо то, что переменная d доступна после блока using, когда уже гарантированно отработал Dispose).

Если тип d спроектирован правильно, то ничего страшного быть не может, так как по контракту IDisposable:

The object must not throw an exception if its Dispose method is called multiple times. Instance methods other than Dispose can throw an ObjectDisposedException when resources are already disposed.

(здесь)

А по-другому иногда не получается: например, прежде чем usaть объяет, надо проверить некоторые условия.

Здравствуйте, _FRED_, Вы писали:

_FR>Здравствуйте, samius, Вы писали:

КЛ>>>Я для себя сделал правилом — объект в using рождается, объект в using и умирает. Внутри никаких манипуляций с ссылкой.
S>>Хорошее правило.
S>>Сам раньше при необходимости пользовался следующим:
_FR>

S>>var d = ...;
S>>using(d)
S>>{
S>>    DoSomething();
S>>}
_FR>

S>>Но сейчас и от этого предостерегают в MSDN (плохо то, что переменная d доступна после блока using, когда уже гарантированно отработал Dispose).

_FR>Если тип d спроектирован правильно, то ничего страшного быть не может, так как по контракту IDisposable:
Да, как раз речь о том, что при обращении к правильно спроектированному классу за пределами блока using наверняка завершится исключением ObjectDisposedException. А объявление переменной вне блока using как раз способствует этому.

_FR>А по-другому иногда не получается: например, прежде чем usaть объяет, надо проверить некоторые условия.
Можно вынести код получения объекта и его проверки во вспомогательный метод, обращение к которому сделать из блока using(var d = GetD())

Здравствуйте, samius, Вы писали:

_FR>>Если тип d спроектирован правильно, то ничего страшного быть не может, так как по контракту IDisposable:
S>Да, как раз речь о том, что при обращении к правильно спроектированному классу за пределами блока using наверняка завершится исключением ObjectDisposedException. А объявление переменной вне блока using как раз способствует этому.

Да, если есть возможность, всегда надо объевлять переменную как можно "локальнее" и как можно длиже к месту использования.

_FR>>А по-другому иногда не получается: например, прежде чем usaть объяет, надо проверить некоторые условия.
S>Можно вынести код получения объекта и его проверки во вспомогательный метод, обращение к которому сделать из блока using(var d = GetD())

Очень может быть. Какой вариант выглядит лучше и почему:

var d = smth();
if(condition(d)) {
  using(d) {
    //…
  }//using
}//if

или

Func<IDisposable> acquire = () => {
  var x = smth();
  return condition(x) ? x : null;
};

using(var d = acquire()) { // Пользуемся тем, что в using->finally есть проверка на null
  if(d != null) {
    //…
  }//if
}//if

?

Здравствуйте, _FRED_, Вы писали:

[]

_FR>Очень может быть. Какой вариант выглядит лучше и почему:

[]

этот

_FR>Func<IDisposable> acquire = () => {
_FR>  var x = smth();
_FR>  return condition(x) ? x : null;
_FR>};

Здравствуйте, _FRED_, Вы писали:

_FR>>>А по-другому иногда не получается: например, прежде чем usaть объяет, надо проверить некоторые условия.
S>>Можно вынести код получения объекта и его проверки во вспомогательный метод, обращение к которому сделать из блока using(var d = GetD())

_FR>Очень может быть. Какой вариант выглядит лучше и почему:
_FR>

_FR>var d = smth();
_FR>if(condition(d)) {
_FR>  using(d) {
_FR>    //…
_FR>  }//using
_FR>}//if
_FR>

_FR>или
_FR>[c#]
_FR>Func<IDisposable> acquire = () => {
_FR> var x = smth();
_FR> return condition(x) ? x : null;
_FR>};
Лучше выглядит — первый (короче, понятнее). Но он добавляет потенциальный риск использования d после освобождения. Конечно, если код довольно полно тестируется, то это не вызовет проблем.

Здравствуйте, samius, Вы писали:

_FR>>Очень может быть. Какой вариант выглядит лучше и почему:
…
S>Лучше выглядит — первый (короче, понятнее). Но он добавляет потенциальный риск использования d после освобождения. Конечно, если код довольно полно тестируется, то это не вызовет проблем.

Указанной проблемы не сложно избежать:

// …
{ // Working with smth resource…
  var d = smth();
  if(condition(d)) {
    using(d) {
      //…
    }//using
  }//if
}
// …

Здравствуйте, _FRED_, Вы писали:

_FR>Очень может быть. Какой вариант выглядит лучше и почему:

Лучше всего выглядит вариант

using (var d = smth())
{
    if (condition(d)) 
    {
        // ...
    }
}

Потому что все прозрачно.

Здравствуйте, amx3000, Вы писали:

_FR>>Очень может быть. Какой вариант выглядит лучше и почему:
A>Лучше всего выглядит вариант

A>using (var d = smth())
A>{
A>    if (condition(d)) 
A>    {
A>        // ...
A>    }
A>}

A>Потому что все прозрачно.

Разница в том, что в моём варианте, в случае невыполнения условия condition не будет вызван Dispose() и объект будет жить дальше. Это условие задачи.

Здравствуйте, _FRED_, Вы писали:

_FR>Здравствуйте, amx3000, Вы писали:

_FR>>>Очень может быть. Какой вариант выглядит лучше и почему:
A>>Лучше всего выглядит вариант

_FR>

A>>using (var d = smth())
A>>{
A>>    if (condition(d)) 
A>>    {
A>>        // ...
A>>    }
A>>}
_FR>

A>>Потому что все прозрачно.

_FR>Разница в том, что в моём варианте, в случае невыполнения условия condition не будет вызван Dispose()

Вообще никогда? Если ниже таки будет вызван, то просто этот код должен быть внутри using после блока if. А если все же никогда, то см. ниже.

_FR>и объект будет жить дальше. Это условие задачи.

Я бы тогда убрал using и вызывал Dispose явно внутри if.
_Лично_я_ считаю, что вне блока using объект, в нем используемый, жить не должен. У него и область видимости-то должна быть ограничена этим блоком. Тогда using четко указывает область жизнь и использования объекта. На мой взгляд, это способствует простоте и понятности кода.

Здравствуйте, amx3000, Вы писали:

_FR>>и объект будет жить дальше. Это условие задачи.

A>Я бы тогда убрал using и вызывал Dispose явно внутри if.

Если вызывать явно, то нужно самому оборачивать в try\finally, чего не хочется.

A>_Лично_я_ считаю, что вне блока using объект, в нем используемый, жить не должен. У него и область видимости-то должна быть ограничена этим блоком. Тогда using четко указывает область жизнь и использования объекта. На мой взгляд, это способствует простоте и понятности кода.

Это подходит только для локальных переменных (и то не всех), но не подходит для полей класса или параметров методов.

В качестве примера такой задачи предлагаю написать функцию, которой передаётся путь к файлу, а вернуть она должна стрим к этому файлу (не закрытый, естественно), но только если первые байты в файле 0x11111111. Если файла нет, или прервые байты в нём не такие, то вернуть следует Stream.Null.

Здравствуйте, _FRED_, Вы писали:

_FR>Если вызывать явно, то нужно самому оборачивать в try\finally, чего не хочется.

Я не про хочется/не хочется, я про прозрачность (=сопровождаемость) кода говорю. Понятное дело, только про мое личное понимание прозрачности кода.

_FR>Это подходит только для локальных переменных (и то не всех), но не подходит для полей класса или параметров методов.

Есть мнение, что нужно проектировать классы и методы таким образом, чтобы не было необходимости использовать using с полями класса или параметрами методов.
Само собой, речь я веду об идеальной ситуации, к которой следует стремиться. А не про реальный мир, в котором обычно все уже спроектировано кем-то, причем далеко не идеально. Увы.

_FR>В качестве примера такой задачи предлагаю написать функцию, которой передаётся путь к файлу, а вернуть она должна стрим к этому файлу (не закрытый, естественно), но только если первые байты в файле 0x11111111. Если файла нет, или прервые байты в нём не такие, то вернуть следует Stream.Null.

Предложение понятно и ответ очевиден — если условия задачи настолько жесткие, то следует делать по-Вашему, либо не использовать using, а использовать try/finally и Dispose вручную.

Но при решении аналогичной задачи я бы все же проектировал классы/методы по-другому, чтобы в подобной функции не возникало необходимости.
То есть вместо

{
    // ...

    стрим = GetПравильныйСтримИлиStreamNull(fileName);
    ДелатьЧтотоСоСтримомКоторыйВполнеМожетБытьStreamNullАМожетИНеБыть(стрим);

    // ...
}

Stream GetПравильныйСтримИлиStreamNull(string fileName)
{
    // тут творим нечто странное и на мой взгляд плохочитаемое c using'ом 
    // и возвращаем то ли FileStream, то ли Stream.Null
}

делал бы что-то вроде

{
    // ...

    if (File.Exists(fileName))
    {
        using (Stream стрим = new FileStream(fileName, FileMode.Open))
        {
            bool isStreamValid = ПроверитьНа0х11111111вНачалеСтрима(стрим);
            if (isStreamValid)
            {
                ДелатьЧтотоСЗаведомоПравильнымСтримом(стрим);
            }
            else
            {
                ДелатьЧтотоСоStreamNullЧтоСамоПоСебеВыглядитНесколькоСтранно(Stream.Null);
            }
        }
    }
    else
    {
        ДелатьЧтотоСоStreamNullЧтоСамоПоСебеВыглядитНесколькоСтранно(Stream.Null);
    }

   // ...
}

Правда, должен признаться, мне этот код тоже не очень нравится (два одинаковых блока else). Навскидку, скорее всего после using следует делать return, блоки else убрать, а их содержимое вынести после блока if. Возможно, посидев, подумав, придумал бы что-то более симпатичное, сейчас времени нет, извините.

Кстати, не подскажете практический пример, для чего нужна была бы работа со Stream.Null? Кроме тестовых целей, как-то ничего в голову не приходит.

Здравствуйте, amx3000, Вы писали:

_FR>>Это подходит только для локальных переменных (и то не всех), но не подходит для полей класса или параметров методов.

A>Есть мнение, что нужно проектировать классы и методы таким образом, чтобы не было необходимости использовать using с полями класса или параметрами методов.

Ты же сам сообщением выше призывал всюду юзингом пользоваться!? Я только заметил, что "везде" пользоваться юзингод нельзя.

A>делал бы что-то вроде

Если, как показано, использовать внутри using, то не получится вернуть из функции закрытый стрим.

A>Кстати, не подскажете практический пример, для чего нужна была бы работа со Stream.Null? Кроме тестовых целей, как-то ничего в голову не приходит.

ИМХО, наоборот. Широко известна рекомендация не возвращать "null" как результат типа string или IEnumerable (а использовать String.Empty и пустой енумератор). Для стримов полезно пользоваться тем же правилом. То есть, когда надо возвратить стрим и по каким-то причинам не удаётся, следует или бросить исключение или вернуть Stream.Null, но не "null".

Здравствуйте, _FRED_, Вы писали:

A>>Есть мнение, что нужно проектировать классы и методы таким образом, чтобы не было необходимости использовать using с полями класса или параметрами методов. :)

_FR>Ты же сам сообщением выше призывал всюду юзингом пользоваться!?

Где это я "призывал всюду юзингом пользоваться"? Просмотрел свои сообщения еще раз — не нашел никаких призывов.
Я писал, что конкретный приведенный выше код плохо выглядит, и после переделки смотрится куда лучше. Еще я писал, что считаю неправильным использовать переменную из юзинга вне этого юзинга. То есть это по определению получается локальная переменная, а не параметр метода или поле класса.

_FR>Я только заметил, что "везде" пользоваться юзингод нельзя.

Я там в предыдущем сообщении предлагал использовать Dispose руками. То есть, с данным утверждением я вполне согласен. Нельзя везде использовать юзинг. Юзинг надо использовать там, где объект должен быть гарантированно уничтожен после его использования. Исходя из этого, использовать объект за границами блока юзинг выглядит несколько странной идеей.

_FR>Если, как показано, использовать внутри using, то не получится вернуть из функции закрытый стрим.

Дык я и говорю — надо сделать так, чтобы не нужно было из метода возвращать стрим — то ли закрытый, то ли открытый, то ли вообще стрим.нулл. Передавать как параметр — бог навстречу, но возвращать... Старое доброе правило — кто (где) создал объект, тот (там) его и уничтожает — считаю очень даже правильным. Такое не всегда возможно, понятно. Но по-моему это должно быть исключением, а не правилом.

_FR>Широко известна рекомендация не возвращать "null" как результат типа string или IEnumerable (а использовать String.Empty и пустой енумератор). Для стримов полезно пользоваться тем же правилом. То есть, когда надо возвратить стрим и по каким-то причинам не удаётся, следует или бросить исключение или вернуть Stream.Null, но не "null".

Понял, спасибо. Правда, есть сомнения в целесообразности подобного (именно со стримом) — с точки зрения производительности. Думаю, ненужная работа со пустым стримом может в некоторых случаях занять ощутимое время. Или оптимизатор это все разруливает? Надо будет покопать по теме при случае, спасибо за наводку.

Здравствуйте, amx3000, Вы писали:

A>>>Есть мнение, что нужно проектировать классы и методы таким образом, чтобы не было необходимости использовать using с полями класса или параметрами методов.
_FR>>Ты же сам сообщением выше призывал всюду юзингом пользоваться!?
A>Где это я "призывал всюду юзингом пользоваться"? Просмотрел свои сообщения еще раз — не нашел никаких призывов.
A>Я писал, что конкретный приведенный выше код плохо выглядит, и после переделки смотрится куда лучше. Еще я писал, что считаю неправильным использовать переменную из юзинга вне этого юзинга. То есть это по определению получается локальная переменная, а не параметр метода или поле класса.

_Лично_я_ считаю, что вне блока using объект, в нем используемый, жить не должен. У него и область видимости-то должна быть ограничена этим блоком. Тогда using четко указывает область жизнь и использования объекта. На мой взгляд, это способствует простоте и понятности кода.

О том, что речь в этих словах идёт о каком-то конкретном юз-кейсе, а не о любом IDisposable-объекте мне, например, было не ясно

A>Понял, спасибо. Правда, есть сомнения в целесообразности подобного (именно со стримом) — с точки зрения производительности. Думаю, ненужная работа со пустым стримом может в некоторых случаях занять ощутимое время.

Например? за счёт чего получится "ощутмость"?