Говнокод #12969 — C# — Говнокод.ру

C# / Говнокод #12969

+136
1. 01
2. 02
3. 03
4. 04
5. 05
6. 06
7. 07
8. 08
9. 09
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, 07 Мая 2013

Tweet
Комментарии (41) RSS
- someone 07.05.2013 10:25 # +8
  
  Где AsParallel?
  Ответить
- tirinox 07.05.2013 12:36 # −2
  
  А по-моему стало более сэкси :)
  Ответить
- vse_govno 07.05.2013 13:35 # 0
  
  Семантичненько.
  Ответить
- Lure Of Chaos 07.05.2013 15:58 # 0
  
  стиль "jQuery" узнается.
  впрочем, стало не менее читаемо
  Ответить
- scriptin 07.05.2013 16:11 # +3
  
  Проблема не в стиле, а в функции CheckAddress - можно проверку на null перенести в нее, и она должна возвращать bool. Тогда оба варианта станут гораздо более читаемыми.
  
  Задумка с chainable API хорошая, реализацию не могу оценить. В целом второй код нравится больше из-за меньшего цикломатического числа и "плоской структуры". Сарказм Sh1tM4ker'a по этому поводу мне непонятен.
  Ответить
  - Steex 07.05.2013 22:41 # 0
    
    Chainable API хорош для циклов. А в приведенном коде я бы лучше весь if вынес в функцию типа "string GetPostAddress(Person person)"
    Ответить
    - neeedle 08.05.2013 07:40 # +2
      
      Узнал автора статьи про реализацию монады maybe на C#.
      Все проверки на null он делает только в методах with, такой вот стиль.
      Насчет говнокода не согласен. Имхо чем короче и компактнее методы, тем лучше, что в принципе тут и показано.
      Whith занимается проверкой на null, If проверяет необходимые условия, Do работает работу, return возвращает значение.
      Ответить
      
      Sh1tM4ker 08.05.2013 10:43 # 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); }); }
      Ответить
      
      LispGovno 08.05.2013 11:30 # 0
      
      > Здесь не ОК
      Многострочные лямбды зло. Вынеси в отдельную функцию и здесь тоже будет ок.
      Ответить
      
      Steex 08.05.2013 12:11 # 0
      
      > Многострочные лямбды зло.
      Не всегда, по-моему. Зависит от количества строк и сложности структуры. В данном случае просто многострочная лямбда накладывается на многострочную цепочку монад, отсюда бардак. А если нужно какой-нибудь обработчик в две-три строки навесить, то вполне нормально это выглядит в виде лямбды.
      Ответить
- LispGovno 08.05.2013 08:29 # +1
  
  > Do
  ForEach
  
  > Return
  Map
  DefaultIfEmpty
  
  Велосипедистам нравятся свои велосипеды
  Ответить
  - neeedle 08.05.2013 10:03 # 0
    
    ForEach() обрабатывает типы реализующие интерфейс IEnumerable, по коду видно, что его там нет.
    Так что вы зря так, насчет велосипеда.
    
    Насчет Map DefaultIfEmpty в статье прямо говорится, что return создан чисто для красоты.
    Ответить
    - LispGovno 08.05.2013 11:28 # 0
      
      > ForEach() обрабатывает типы реализующие интерфейс IEnumerable, по коду видно, что его там нет.
      Ты не поверишь, но после With IEnumerable там есть.
      Ответить
      
      neeedle 08.05.2013 12:08 # 0
      
      Насколько я понимаю Address - не Ienumerable.
      Ответить
      
      LispGovno 08.05.2013 12:25 # −1
      
      А адрес тут причем?
      With(x => x.Address) - Ienumerable
      Ответить
      
      neeedle 08.05.2013 12:30 # 0
      
      С чего это вдруг?
      Вернется либо null либо address
      Ответить
      
      LispGovno 08.05.2013 13:01 # 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); }
      Примерно так.
      Ответить
      
      LispGovno 08.05.2013 13:03 # −1
      
      Впрочем да. Велосипедисты любят писать ненужный код.
      > .If(x => HasMedicalRecord(x))]
      > If
      Where
      Ответить
      
      LispGovno 08.05.2013 13:08 # −1
      
      > return Enumerable.Repeat(evaluator(o), 1);
      Достойно отдельного говнокода, так что примерно
      
      (new TResult[] {evaluator(o)}).asEnumerable()
      возможно лучше.
      И o забыл развернуть из IEnumerable:
      Ответить
      
      LispGovno 08.05.2013 13:09 # −1
      
      evaluator(o.Single())
      Ответить
      
      LispGovno 08.05.2013 13:39 # 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
      Ответить
      
      3.14159265 08.05.2013 14:45 # +2
      
      Не знаю кто гумна минусует, но здравое зерно в использовании стандартного ленивого спи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"); }
      Ответить
      
      neeedle 08.05.2013 19:37 # 0
      
      Какой IEnumerable, вы чего городите огород?
      В посте рассматривается операция с одной записью person, у которой есть одна запись address?
      Эту задачу можно решить с помощью стандартного кода, а можно сделать его более удобочитаемым и приятным на вид. Оба кода делают одно и тоже, просто в первом случае создается решение в лоб, во втором человек проявляет некую изобретательность.
      Ваше право использовать его способ или нет, но не надо называть его велосипедистом и городить вот эти костыли.
      Ответить
      
      3.14159265 08.05.2013 20:18 # +1
      
      >Какой IEnumerable, вы чего городите огород?
      Огород нагородил любитель монадического синтаксиса. Использовать лямбды только чтобы покрыть проверкой одну переменную - глупо.
      Даже @LispGovno понимает где есть один person, там могут быть миллионы.
      
      > но не надо называть его велосипедистом и городить вот эти костыли.
      Какие я горожу костыли? Где я назвал его велосипедом?
      И с чего такая болезненная реакция "не надо называть его велосипедистом"? Вы автор штоле?
      Ответить
      
      neeedle 08.05.2013 20:31 # 0
      
      1) Да все это понимают. Вы значение слова "пример" понимаете?
      2) Обычный костыли. Самые настоящие костыли. Вы подстраиваете задачу под код, а не код под задачу.
      Туше мсье кот.
      Ответить
      
      3.14159265 08.05.2013 20:41 # 0
      
      >Вы значение слова "пример" понимаете?
      Я не понимаю Вас.
      >Обычный костыли. Самые настоящие костыли. Вы подстраиваете задачу под код, а не код под задачу.
      Сумасшедий?
      http://govnokod.ru/12969#comment176506
      Где тут костыли, neeedle?!
      Ответить
      
      neeedle 09.05.2013 07:21 # 0
      
      1) Ну видимо и не поймешь.
      2) Сам такой, я имел ввиду не код, а одобрение ленивого списка. В задаче идет проверка одного конкретного person'a, вы суете туда IEnumerable. Без мыла в жопу.
      Ответить
      
      3.14159265 10.05.2013 15:32 # +3
      
      >вы суете туда IEnumerable
      Я не использовал IEnumerable, равно как я не писал слова "велосипед". Опять вы спутали person.Address.
      
      Ну сделал он ленивый список из одного элемента. И что? Оверхед:? Да использование лямбд уже оверхед. И написав => вы подписываетесь что вам плевать на оптимальность.
      
      Я лишь сказал что не стоит минусовать человека только за то, что он использует стандартный интерфейс и функционал, вместо самописного и сделал код чуток универсальнее.
      Тем более что в нике у него стоит LISP.
      Ответить
      
      neeedle 10.05.2013 17:03 # 0
      
      Как горох об стену.
      Это бессмысленно.
      Ответить
      
      roman-kashitsyn 09.05.2013 13:19 # +3
      
      В Scala я бы выпилил отовсюду null, заменив их на Option, получилось бы в итоге что-то вроде
      person.filter(p => p.address.isDefined && hasMedicalRecord(p)) .map(checkAddress(_.address)) // let checkAddress return checked address .map(_.postCode).getOrElse("UNKNOWN")
      CheckAddress меня смущает, подозреваю, что он кидает исключение в случае невалидного адреса. Как-то не очень вписывается в контекстный способ "мочаливой" обработки ошибок.
      Ответить
      
      LispGovno 09.05.2013 21:17 # 0
      
      > В Scala я бы выпилил отовсюду null, заменив их на Option
      На Box ты хотел сказать?
      Ответить
      
      roman-kashitsyn 09.05.2013 22:22 # 0
      
      Box это класс из Lift, в Scala есть встроенный Option
      Ответить
      
      LispGovno 09.05.2013 22:40 # 0
      
      Оптин более ограниченный. Box лучше вписывается в функциональщину
      Ответить
      
      scriptin 09.05.2013 23:46 # 0
      
      KISS
      Ответить
      
      LispGovno 10.05.2013 11:48 # −3
      
      Что это ты в меня кисом тычишь? А когда вдруг в модуле проекта понадобится функционал Box, а он обязательно понадобится в большом проекте, то ты во всем проекте Оптин на Бокс будешь заменять?
      Ответить
      
      roman-kashitsyn 10.05.2013 12:10 # +1
      
      Box, разумеется, заслуживает того, чтобы ради него тянуть в classpath liftweb.
      Ответить
      
      LispGovno 10.05.2013 13:31 # 0
      
      Неужели ты не согласен с тем, что оптин реальное говно в то время как бокс нереально хорош?
      Ответить
      
      scriptin 10.05.2013 14:25 # +1
      
      >а он обязательно понадобится
      YAGNI
      Ответить
      
      LispGovno 10.05.2013 15:06 # −2
      
      Ты даже не знаешь что этот принцип декларирует.
      
      Принцип «YAGNI» — процесс и принцип проектирования, при котором в качестве основной цели и/или ценности декларируется отказ от избыточной функциональности.
      
      То есть не добавляй функционал к программе, который никому не нужен и не понадобится.
      Ответить
      
      3.14159265 10.05.2013 15:14 # +2
      
      >декларируется отказ от избыточной функциональности.
      >не добавляй функционал к программе, который никому не нужен
      Теперь ему осталось ответить только:
      DRY
      Ответить
      
      LispGovno 10.05.2013 15:34 # 0
      
      Я повторился на тот случай, если он не способен понять.
      Don't replicate yourself
      И не собирался. Если бы я создавал клонов, то они стали мне конкурентами. Лучше я единолично захвачу мир.
      Ответить
Добавить комментарий
Ошибка компиляции комментария:

Гости могут высказаться только в понедельник, среду, четверг или воскресение
Семь раз отмерь — один отрежь, guest!

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

[b]жирный[/b] — жирный

[i]курсив[/i] — курсив

[u]подчеркнутый[/u] — подчеркнутый

[s]перечеркнутый[/s] — перечеркнутый

[blink]мигающий[/blink] — мигающий

[color=red]цвет[/color] — цвет (подробнее)

[size=20]размер[/size] — размер (подробнее)

[code=<language>]some code[/code] (подробнее)

Проверочный код: *

Говнокод: по колено в коде.

C# / Говнокод #12969

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

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