Си диез / Говнокод #12969 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
  6. 6
  7. 7
  8. 8
  9. 9
  10. 10
  11. 11
  12. 12
  13. 13
  14. 14
  15. 15
  16. 16
  17. 17
  18. 18
  19. 19
  20. 20
// Было
string postCode;
if (person != null)
{
  if (HasMedicalRecord(person) && person.Address != null)
  {
    CheckAddress(person.Address);
    if (person.Address.PostCode != null)
      postCode = person.Address.PostCode.ToString();
    else
      postCode = "UNKNOWN";
  }
}
// Стало
string postCode = this.With(x => person)
    .If(x => HasMedicalRecord(x))]
    .With(x => x.Address)
    .Do(x => CheckAddress(x))
    .With(x => x.PostCode)
    .Return(x => x.ToString(), "UNKNOWN");

"как можно использовать более “монадический” синтаксис в C# для того, чтобы __повысить удобочитаемость__ исходного кода"
http://www.gotdotnet.ru/blogs/nesteruk/6975/

Запостил: Sh1tM4ker Sh1tM4ker, (Updated )

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

  • ava scriptin # Ссылка на оригинал 0

    Проблема не в стиле, а в функции CheckAddress - можно проверку на null перенести в нее, и она должна возвращать bool. Тогда оба варианта станут гораздо более читаемыми.

    Задумка с chainable API хорошая, реализацию не могу оценить. В целом второй код нравится больше из-за меньшего цикломатического числа и "плоской структуры". Сарказм Sh1tM4ker'a по этому поводу мне непонятен.
    Ответить

    • ava Steex # Ссылка на оригинал 0 ⇈

      Chainable API хорош для циклов. А в приведенном коде я бы лучше весь if вынес в функцию типа "string GetPostAddress(Person person)"
      Ответить

      • ava neeedle # Ссылка на оригинал 0 ⇈

        Узнал автора статьи про реализацию монады maybe на C#.
        Все проверки на null он делает только в методах with, такой вот стиль.
        Насчет говнокода не согласен. Имхо чем короче и компактнее методы, тем лучше, что в принципе тут и показано.
        Whith занимается проверкой на null, If проверяет необходимые условия, Do работает работу, return возвращает значение.
        Ответить

        • ava Sh1tM4ker # Ссылка на оригинал 0 ⇈

          Пока писал коммент про то, что читаемость не повысилась, привык к этому стилю и наметал глаз. Теперь выцепляет только нужное.
          В принципе, в отдельных случаях вполне потребно.
          Минусы, из-за которых в отличных от "отдельных случаях" такой метод использовать неудобно, описал автор статьи.
          Из запоминающихся про отладку: "Поэтому во многих случаях мой метод работы с подобным интерфейсом такой: сначала я пишу его в “обычном” стиле, потом делаю отладку, и только когда я уверен что код работает я переписываю его в “монадическом” стиле."

          Моё субъективное мнение:
          Здесь ОК:
          string typeName = this.With(x => expression)
    .With(x => x.InvokedExpression)
    .With(x => x as IReferenceExpression)
    .With(x => x.Reference)
    .With(x => x.Resolve())
    .With(x => x.DeclaredElement)
    .With(x => x.GetContainingType())
    .Return(x => x.CLRName, null);

          Здесь не ОК, лучше по-старинке:
          this.If(x => Array.IndexOf(types, typeName) != -1)
    .With(x => ExpressionStatementNavigator.GetByEx * pression(expression))
    .Do(x =>
          {
            var suggestion = new SideEffectSuggestion(typeName);
            var highlightInfo = new HighlightingInfo(
              expression.GetDocumentRange(),
              suggestion);
            context.HighlightingInfos.Add(highlightInfo);
          });
}
          Ответить

          • ava LispGovno # Ссылка на оригинал 0 ⇈

            > Здесь не ОК
            Многострочные лямбды зло. Вынеси в отдельную функцию и здесь тоже будет ок.
            Ответить

            • ava Steex # Ссылка на оригинал 0 ⇈

              > Многострочные лямбды зло.
              Не всегда, по-моему. Зависит от количества строк и сложности структуры. В данном случае просто многострочная лямбда накладывается на многострочную цепочку монад, отсюда бардак. А если нужно какой-нибудь обработчик в две-три строки навесить, то вполне нормально это выглядит в виде лямбды.
              Ответить

  • ava LispGovno # Ссылка на оригинал 0

    > Do
    ForEach

    > Return
    Map
    DefaultIfEmpty

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

    • ava neeedle # Ссылка на оригинал 0 ⇈

      ForEach() обрабатывает типы реализующие интерфейс IEnumerable, по коду видно, что его там нет.
      Так что вы зря так, насчет велосипеда.

      Насчет Map DefaultIfEmpty в статье прямо говорится, что return создан чисто для красоты.
      Ответить

      • ava LispGovno # Ссылка на оригинал 0 ⇈

        > ForEach() обрабатывает типы реализующие интерфейс IEnumerable, по коду видно, что его там нет.
        Ты не поверишь, но после With IEnumerable там есть.
        Ответить

              • ava LispGovno # Ссылка на оригинал 0 ⇈

                > Вернется либо null либо address
                Только у велосипедистов.
                public static IEnumerable<TResult> With<TInput, TResult>(this TInput o, Func<TInput, TResult> evaluator)
  where TResult : class where TInput : class
{
  if (o == null) return Enumerable.Empty<TResult>();
  return Enumerable.Repeat(evaluator(o), 1);
}
                Примерно так.
                Ответить

                • ava LispGovno # Ссылка на оригинал 0 ⇈

                  Впрочем да. Велосипедисты любят писать ненужный код.
                  > .If(x => HasMedicalRecord(x))]
                  > If
                  Where
                  Ответить

                • ava LispGovno # Ссылка на оригинал 0 ⇈

                  > return Enumerable.Repeat(evaluator(o), 1);
                  Достойно отдельного говнокода, так что примерно
                  (new TResult[] {evaluator(o)}).asEnumerable()
                  возможно лучше.
                  И o забыл развернуть из IEnumerable:
                  Ответить

                    • ava LispGovno # Ссылка на оригинал 0 ⇈ 

                      string postCode = this.EnumerableOfSingleOrEmptyIfNull()
    .SelectOrEmptyIfNull(x => person)
    .Where(x => HasMedicalRecord(x))]
    .SelectOrEmptyIfNull(x => x.Address)
    .ForEach(x => CheckAddress(x))
    .SelectOrEmptyIfNull(x => x.PostCode)
    .SelectOrEmptyIfNull(x => x.ToString())
    .SingleOrDefault("UNKNOWN");

                      SelectOrEmptyIfNull и EnumerableOfSingleOrEmptyIfNull также пишутся как композиция стандартных HOF
                      Ответить

                • ava 3.14159265 # Ссылка на оригинал 0 ⇈

                  Не знаю кто гумна минусует, но здравое зерно в использовании стандартного ленивого спиcка (IEnumerable) есть.
                  C другой стороны неймнинг в посте короткий и читабельный, а стандартный - длииноват.

                  С третьей стороны, метод ToString(Object o, String whenNull) - практически мастхев, всё-равно вы эту же статику лепите.
                  Потому я бы писал так:
                  if (       person != null 
    && person.Address != null
    && HasMedicalRecord(person) 
){
    CheckAddress(person.Address);
 return ToString(person.Address.PostCode,"UNKNOWN");
  }
                  Ответить

                  • ava neeedle # Ссылка на оригинал 0 ⇈

                    Какой IEnumerable, вы чего городите огород?
                    В посте рассматривается операция с одной записью person, у которой есть одна запись address?
                    Эту задачу можно решить с помощью стандартного кода, а можно сделать его более удобочитаемым и приятным на вид. Оба кода делают одно и тоже, просто в первом случае создается решение в лоб, во втором человек проявляет некую изобретательность.
                    Ваше право использовать его способ или нет, но не надо называть его велосипедистом и городить вот эти костыли.
                    Ответить

                    • ava 3.14159265 # Ссылка на оригинал 0 ⇈

                      >Какой IEnumerable, вы чего городите огород?
                      Огород нагородил любитель монадического синтаксиса. Использовать лямбды только чтобы покрыть проверкой одну переменную - глупо.
                      Даже @LispGovno понимает где есть один person, там могут быть миллионы.

                      > но не надо называть его велосипедистом и городить вот эти костыли.
                      Какие я горожу костыли? Где я назвал его велосипедом?
                      И с чего такая болезненная реакция "не надо называть его велосипедистом"? Вы автор штоле?
                      Ответить

                      • ava neeedle # Ссылка на оригинал 0 ⇈

                        1) Да все это понимают. Вы значение слова "пример" понимаете?
                        2) Обычный костыли. Самые настоящие костыли. Вы подстраиваете задачу под код, а не код под задачу.
                        Туше мсье кот.
                        Ответить

                        • ava 3.14159265 # Ссылка на оригинал 0 ⇈

                          >Вы значение слова "пример" понимаете?
                          Я не понимаю Вас.
                          >Обычный костыли. Самые настоящие костыли. Вы подстраиваете задачу под код, а не код под задачу.
                          Сумасшедий?
                          http://govnokod.ru/12969#comment176506
                          Где тут костыли, neeedle?!
                          Ответить

                          • ava neeedle # Ссылка на оригинал 0 ⇈

                            1) Ну видимо и не поймешь.
                            2) Сам такой, я имел ввиду не код, а одобрение ленивого списка. В задаче идет проверка одного конкретного person'a, вы суете туда IEnumerable. Без мыла в жопу.
                            Ответить

                            • ava 3.14159265 # Ссылка на оригинал 0 ⇈

                              >вы суете туда IEnumerable
                              Я не использовал IEnumerable, равно как я не писал слова "велосипед". Опять вы спутали person.Address.

                              Ну сделал он ленивый список из одного элемента. И что? Оверхед:? Да использование лямбд уже оверхед. И написав => вы подписываетесь что вам плевать на оптимальность.

                              Я лишь сказал что не стоит минусовать человека только за то, что он использует стандартный интерфейс и функционал, вместо самописного и сделал код чуток универсальнее.
                              Тем более что в нике у него стоит LISP.
                              Ответить

                  • ava roman-kashitsyn # Ссылка на оригинал 0 ⇈

                    В Scala я бы выпилил отовсюду null, заменив их на Option, получилось бы в итоге что-то вроде 
                    person.filter(p => p.address.isDefined && hasMedicalRecord(p))
      .map(checkAddress(_.address)) // let checkAddress return checked address
      .map(_.postCode).getOrElse("UNKNOWN")
                    CheckAddress меня смущает, подозреваю, что он кидает исключение в случае невалидного адреса. Как-то не очень вписывается в контекстный способ "мочаливой" обработки ошибок.
                    Ответить

                    • ava LispGovno # Ссылка на оригинал 0 ⇈

                      > В Scala я бы выпилил отовсюду null, заменив их на Option
                      На Box ты хотел сказать?
                      Ответить

                        • ava LispGovno # Ссылка на оригинал 0 ⇈

                          Оптин более ограниченный. Box лучше вписывается в функциональщину
                          Ответить

                            • ava LispGovno # Ссылка на оригинал 0 ⇈

                              Что это ты в меня кисом тычишь? А когда вдруг в модуле проекта понадобится функционал Box, а он обязательно понадобится в большом проекте, то ты во всем проекте Оптин на Бокс будешь заменять?
                              Ответить

                              • ava roman-kashitsyn # Ссылка на оригинал 0 ⇈

                                Box, разумеется, заслуживает того, чтобы ради него тянуть в classpath liftweb.
                                Ответить

                                • ava LispGovno # Ссылка на оригинал 0 ⇈

                                  Неужели ты не согласен с тем, что оптин реальное говно в то время как бокс нереально хорош?
                                  Ответить

                                • ava LispGovno # Ссылка на оригинал 0 ⇈

                                  Ты даже не знаешь что этот принцип декларирует.

                                  Принцип «YAGNI» — процесс и принцип проектирования, при котором в качестве основной цели и/или ценности декларируется отказ от избыточной функциональности.

                                  То есть не добавляй функционал к программе, который никому не нужен и не понадобится.
                                  Ответить

                                  • ava 3.14159265 # Ссылка на оригинал 0 ⇈

                                    >декларируется отказ от избыточной функциональности.
                                    >не добавляй функционал к программе, который никому не нужен
                                    Теперь ему осталось ответить только:
                                    DRY
                                    Ответить

                                    • ava LispGovno # Ссылка на оригинал 0 ⇈

                                      Я повторился на тот случай, если он не способен понять.
                                      Don't replicate yourself
                                      И не собирался. Если бы я создавал клонов, то они стали мне конкурентами. Лучше я единолично захвачу мир.
                                      Ответить

Добавить комментарий

Переведи на "PHP", guest!

    А не использовать ли нам bbcode?

    ↑
    8
    ↓