company_banner

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

Original author: Raymond Chen
  • Translation
В С# существует два способа преобразования объектов: использовать оператор 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();

Only registered users can participate in poll. Log in, please.

Переводить ли в дальнейшем подобные заметки Рэймонда Чена из блога The Old New Thing?

Microsoft
125.80
Microsoft — мировой лидер в области ПО и ИТ-услуг
Share post

Comments 43

    +2

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

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

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

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

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

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

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

              +1

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

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

                Я легко могу представить linq-провайдера, который первый вариант компилирует в
                select count(*) from t where ...
                а второй — в
                select count(case when ... then 1 else null end) from t
        –6
        А вы отсутствие объектов в коллекциях всегда через отлов Exception определяете что-ли? Вышеприведенное использование firstordefault и AS не соответствует их реальному назначению.
          –5
          И вообще зачем тут Linq приплетать, если вы там даже условие не прописываете. Где вариант с [0]?
          +1
          второй
            +1
            NullReferenceException может быть и из-за того, то самой коллекции не существует. Именно поэтому второй вариант предпочтительней. Особенно в реалиях UWP, где callstack в большинстве случаев весьма неинформативен
              +1
              Вы же знаете про StackParser из https://github.com/dotnet/corefx-tools/?
                +1
                Проблема в том, что очень часто StackTrace просто пустой. Я говорю не о 100%, а об ошибках, которые не покрыты локальными try..catch, а те, что можно отловить уже через Application.UnhandledException
                  +1
                  Чаще всего это деббагер глючит и если вывести ошибку в мессаджбокс, то трейс магическим образом появляется
                    +1
                    Какой еще дебагер в Release? Я говорю о том, что в Exception.StackTrace, который приходит в Application.UnhandledException, часто бывает просто пустой
                      +1
                      А вот это уже же странно. В логи трейс обычно попадает. Видно есть в вашем коде что-то конкретное, что трет трейс.
                        +1
                        Данная проблема действительно существует. Возможно, заметка Вам поможет:
                          +1
                          тут на самом деле может быть ситуация, что проблема вообще в стороннем нативном коде…
                            +1
                            Спасибо. Заметка то что нужно. Надо будет попробовать решение «на досуге»
                  +1
                  Это, извините, очень слабая статья. Приведенные примеры не про отладку — они про использование подходящих инструкций. Если вы используете методы, не выбрасывающие исключение — то проверяйте возвращаемый результат (если он может быть невалидным). Я понимаю, корпоративный блог и всё такое, но зачем настолько куцые материалы переводить?
                  А вот про именно специфичные для отладки/инвестигирования кейсы было интересно почитать, даже сходу ничего не приходит в голову.
                    +6
                    // вариант 3
                    switch (thing)
                    {
                       case Foo foo:
                            foo.DoSomething();
                    .....
                    }
                    

                    Из разряда «из двух неправильных вариантов выберите менее неправильный»
                      +2
                      Да, новый C# сильно помогает в таких делах.
                        +2
                        В Вашем варианте тогда нужен default обработчик, где Вы что будете делать? Скорее всего надо кидать исключение, если ни один из case не заматчил объект. Или просто проглотите и ничего не сделаете?
                          +2
                          Да, исключение. И, возможно, более осмысленное, нежели NRE или ошибка приведения типов. Но, надо отдать должное автору, контекста в данном вопросе — ноль. Так что, спорить что лучше я не буду :)
                            +2

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

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

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

                                  +1
                                  Конечно точно, это ж мой пример :)
                          +1
                          Все варианты с явным приведением типов — неправильные.
                          –1

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

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

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


                              collection.FirstOrDefault()?.DoSomething();

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

                              +2

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


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


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

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

                                    First
                                    image
                                      0

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


                                      var item = collection.FirstOrDefault().DoSomething();
                                      
                                      if(item == null)
                                      {
                                         // обработать
                                      }
                                      
                                        +2
                                        До if вы дойдете только если в коллекции есть элементы.
                                          +1

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


                                          var item = collection.FirstOrDefault();
                                          
                                          if(item==null)
                                          {
                                           // Обработать
                                          }
                                          
                                          item.DoSomething();
                                        +1

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


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

                                            Странно, но по мне разница есть. И она описывает соглашение того, что мы ожидаем получить в thing.


                                            Использую оба варинта:


                                            • вариант 1 — если в thing может быть произвольного типа.
                                            • вариант 2 — если в thing обязан реализовать контракт типа Foo

                                            И да, проверка на null — для обоих результатов обязательна, с выбрасыванием "вменяемого" исключения.

                                              0

                                              Это вы сравнили варианты 3 и 4. В вариантах 1 и 2 проверки на null нету.

                                            Only users with full accounts can post comments. Log in, please.