27.07.2017 13:37
Schvepsss 41 6300 Источник
В С# существует два способа преобразования объектов: использовать оператор as, который пытается преобразовать объект и в случае успеха возвращает результат, в случае неудачи null; или использовать оператор преобразования.



Какой из этих вариантов выбрать, когда нужно немедленно воспользоваться результатом преобразования?

// вариант 1
var thing = GetCurrentItem();
var foo = thing as Foo;
foo.DoSomething();


// вариант 2
var thing = GetCurrentItem();
var foo = (Foo)thing;
foo.DoSomething();

Теперь предположим, что объект thing не является типом Foo. Оба варианта отработают некорректно, однако сделают они это по-разному.

В первой варианте отладчик выдаст исключение NullReferenceException в методе foo.Do­Something(), и дамп сбоя подтвердит, что переменная foo является null. Однако этого может и не быть в дампе сбоя. Возможно, дамп сбоя захватывает лишь параметры, которые участвовали в выражении, которое в свою очередь привело к исключению. Или может быть переменная thing попадёт в сборщик мусора. Вы не можете определить где проблема когда GetCurrentItem возвращает null или GetCurrentItem возвращает объект другого типа, отлично от типа Foo. И что это если не Foo?

Во втором варианте также могут возникнуть ошибки. Если объект thing является null, при вызове метода foo.Do­Something() ты получишь исключение NullReferenceException. Однако, если объект thing имеет другой тип, сбой произойдет в точке преобразования типов и выдаст исключение InvalidCastException. Если повезёт, отладчик покажет что именно нельзя преобразовать. Если не сильно повезёт, можно будет определить где произошел сбой по типу выданного исключения.

Задание: Оба варианта ниже функционально эквивалентны. Какой будет проще отладить?

// вариант 1
collection.FirstOrDefault().DoSomething();


// вариант 2
collection.First().DoSomething();
Переводить ли в дальнейшем подобные заметки Рэймонда Чена из блога The Old New Thing?

Проголосовало 173 человека. Воздержалось 67 человек.

Только зарегистрированные пользователи могут участвовать в опросе. Войдите, пожалуйста.

Поделиться с друзьями
-->

Комментарии (41)


  1. AgentFire
    27.07.2017 17:33
    #10331150
    +2

    Очевидно, второй.
    Какие-то детские задачки :S


    1. Schvepsss
      27.07.2017 17:35
      #10331166
      +1

      Мы параллельно запустили голосовалки в соц. сетях, в Telegram пока побеждает первый (65%).


    1. mayorovp
      27.07.2017 20:00
      #10331492
      +3

      Тем не менее, я почему-то очень часто вижу именно первый вариант в чужом коде...


      1. qw1
        28.07.2017 00:08
        #10331838
        +3

        У меня такие же наблюдения…

        Программисты выучивают FirstOrDefault() и используют его везде, даже где более уместны First() или ещё лучше Single().

        Используют .Where(predicate).Count() вместо .Count(predicate), или какие-то запутанные условия в OrderBy вместо ThenBy, не используют Cast() и GroupBy()


        1. AgentFire
          28.07.2017 21:57
          #10333760
          +1

          Используют .Where(predicate).Count() вместо .Count(predicate)
          где то я читал, что первый вариант оптимальней


          1. mayorovp
            29.07.2017 12:08
            #10334244
            +1

            Каким образом вариант, создающий лишнюю обертку, может быть оптимальнее?


            1. qw1
              29.07.2017 12:41
              #10334280
              +1

              Вероятно, дело в кривом поставщике данных, который не оптимизирует некоторые IQueryable.

              Я легко могу представить linq-провайдера, который первый вариант компилирует в
              select count(*) from t where ...
              а второй — в
              select count(case when ... then 1 else null end) from t


  1. piranuy
    27.07.2017 17:34
    #10331160
    -6

    А вы отсутствие объектов в коллекциях всегда через отлов Exception определяете что-ли? Вышеприведенное использование firstordefault и AS не соответствует их реальному назначению.


    1. piranuy
      27.07.2017 17:35
      #10331168
      -5

      И вообще зачем тут Linq приплетать, если вы там даже условие не прописываете. Где вариант с [0]?


  1. DiamondBK
    27.07.2017 17:36
    #10331170
    +1

    второй


  1. alexeystarchikov
    27.07.2017 17:40
    #10331184
    +1

    NullReferenceException может быть и из-за того, то самой коллекции не существует. Именно поэтому второй вариант предпочтительней. Особенно в реалиях UWP, где callstack в большинстве случаев весьма неинформативен


    1. ad1Dima
      28.07.2017 06:41
      #10331998
      +1

      Вы же знаете про StackParser из https://github.com/dotnet/corefx-tools/?


      1. alexeystarchikov
        28.07.2017 09:51
        #10332150
        +1

        Проблема в том, что очень часто StackTrace просто пустой. Я говорю не о 100%, а об ошибках, которые не покрыты локальными try..catch, а те, что можно отловить уже через Application.UnhandledException


        1. ad1Dima
          28.07.2017 09:54
          #10332162
          +1

          Чаще всего это деббагер глючит и если вывести ошибку в мессаджбокс, то трейс магическим образом появляется


          1. alexeystarchikov
            28.07.2017 10:14
            #10332196
            +1

            Какой еще дебагер в Release? Я говорю о том, что в Exception.StackTrace, который приходит в Application.UnhandledException, часто бывает просто пустой


            1. ad1Dima
              28.07.2017 10:18
              #10332202
              +1

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


            1. strayker1206
              28.07.2017 10:28
              #10332232
              +1

              Данная проблема действительно существует. Возможно, заметка Вам поможет:


              1. ad1Dima
                28.07.2017 10:36
                #10332256
                +1

                тут на самом деле может быть ситуация, что проблема вообще в стороннем нативном коде…


              1. alexeystarchikov
                28.07.2017 12:38
                #10332538
                +1

                Спасибо. Заметка то что нужно. Надо будет попробовать решение «на досуге»


  1. DrFdooch
    27.07.2017 17:40
    #10331186
    +1

    Это, извините, очень слабая статья. Приведенные примеры не про отладку — они про использование подходящих инструкций. Если вы используете методы, не выбрасывающие исключение — то проверяйте возвращаемый результат (если он может быть невалидным). Я понимаю, корпоративный блог и всё такое, но зачем настолько куцые материалы переводить?
    А вот про именно специфичные для отладки/инвестигирования кейсы было интересно почитать, даже сходу ничего не приходит в голову.


  1. aosja
    27.07.2017 17:40
    #10331194
    +6

    // вариант 3
switch (thing)
{
   case Foo foo:
        foo.DoSomething();
.....
}

    Из разряда «из двух неправильных вариантов выберите менее неправильный»


    1. KReal
      27.07.2017 18:38
      #10331320
      +2

      Да, новый C# сильно помогает в таких делах.


    1. blanabrother
      27.07.2017 18:47
      #10331334
      +2

      В Вашем варианте тогда нужен default обработчик, где Вы что будете делать? Скорее всего надо кидать исключение, если ни один из case не заматчил объект. Или просто проглотите и ничего не сделаете?


      1. aosja
        27.07.2017 22:38
        #10331710
        +2

        Да, исключение. И, возможно, более осмысленное, нежели NRE или ошибка приведения типов. Но, надо отдать должное автору, контекста в данном вопросе — ноль. Так что, спорить что лучше я не буду :)


        1. mayorovp
          28.07.2017 05:50
          #10331978
          +2

          Что может быть осмысленнее ошибки приведения типа в ситуации когда у объекта неправильный тип?


          1. aosja
            28.07.2017 09:53
            #10332158
            +2

            Например, что поддержка такого-то алгоритма/бизнес-процесса и т.п. еще не реализована.


            1. mayorovp
              28.07.2017 09:56
              #10332166
              +1

              А она точно "еще не реализована"?


              1. aosja
                28.07.2017 10:09
                #10332184
                +1

                Конечно точно, это ж мой пример :)


    1. IL_Agent
      27.07.2017 20:19
      #10331524
      +1

      Все варианты с явным приведением типов — неправильные.


  1. Mispon
    27.07.2017 18:08
    #10331254
    -1

    Предпочтительнее тот вариант, который больше удовлетворяет цель. Если мы, например, для того, чтобы отобразить на главной странице незначительную ерунду используем .Firts() и весь веб падает в случае отсутствии этой ерунды, то аргумент "зато проще отлаживать" явно не катит.


    1. unsafePtr
      27.07.2017 19:11
      #10331376
      +1

      First, FirstOrDefault, Single, SingleOrDefault очень хорошо улучшают читаемость кода. Согласен что надо искать компромисс между читаемостью и производительнотю, и шарпам это очень хорошо удаётся. Кстати говоря, если метод не обязательно должен быть исполнен мы можем воспользоваться элвис оператором ?.


    1. Serg046
      28.07.2017 00:49
      #10331864
      +2

      В приведенном примере, в случае отсутствия элемента в кол-ции, FirstOrDefault() все равно не спасает.
      Тогда уж его хотя бы нужно переписать:


      collection.FirstOrDefault()?.DoSomething();

      Так что вариант 1 просто вносит дополнительную ненужную проверку и портит колстек в случае ошибки.


  1. Andikki
    27.07.2017 21:06
    #10331588
    +2

    Глупость какая-то. В обоих случаях первый вариант является просто плохим кодом. Когда пишешь thing as Foo или collection.FirstOrDefault(), нужно учесть вероятность получения null — проверить результат на null, использовать оператор ?. и т. п.


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


    Вопрос выбора между равнозначными вариантами не стоит.


  1. stranger777
    27.07.2017 21:14
    #10331598
    +4

    Сначала ты работаешь на имя — потом имя работает на тебя.
    Представим себе, что ровно тот же текст, байт в байт, только без картинко-текста (пора вводить в язык новое слово — pictext, оно же пиктекст) с логотипом Майкрософт, Вася Пупкин написал в песочницу. Вопрос: пустят ли Васю Пупкина на Хабр?
    Ну, нельзя так… а проще, конечно, второй. Если что. Это можно ответить даже не зная C#.


  1. oleg2
    27.07.2017 22:11
    #10331680
    +1

    Конечно, второй)
    Хотя здесь все-таки не NullReferenceException, а ArgumentNullException, First будет как минимум более выразительным.


  1. morozyan
    27.07.2017 22:12
    #10331682
    +1

    С первого взгляда, второй проще отладить, но если углубиться в задачу, то все равно второй вариант выигрывает:

    FirstOrDefault
    image


  1. Alex_ME
    28.07.2017 01:58
    #10331900

    Смотря для чего лучше.
    С точки зрения отладки, второй лучше — если мы забыли проверку, он вывалится в одном месте, а не в рандомном, где мы используем результат. Но с точки зрения производительности лучше просто проверить результат на null.


    var item = collection.FirstOrDefault().DoSomething();

if(item == null)
{
   // обработать
}


    1. ad1Dima
      28.07.2017 06:46
      #10332002
      +2

      До if вы дойдете только если в коллекции есть элементы.


      1. Alex_ME
        28.07.2017 15:22
        #10332990
        +1

        Да, я ошибся, криво скопировав эту строчку из статьи.
        Имел в виду это:


        var item = collection.FirstOrDefault();

if(item==null)
{
 // Обработать
}

item.DoSomething();


  1. Atomosk
    28.07.2017 08:56
    #10332078
    +1

    Еще никогда у меня не было выбора между двумя вариантами кода, вариантов всегда очень много. Пока есть дженерики + экстеншны или наследование первые 2 варианта примера никогда не придется писать.


    Оба варианта ниже функционально эквивалентны.
    В общем случае нет.


  1. gset
    28.07.2017 10:01
    #10332172
    +1

    Как по мне, то решать надо исходя из ситуации. Если наличие null в место объекта не повредит программе, то логичнее применять as. Если же null не желателен, то кастуем. Для себя давно решил, что as у меня применяется только на верхнем уровне(например в controller mvc).