1. Objective C / Говнокод #14369

    −104

    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
    21. 21
    22. 22
    -(BOOL) wasChanged
{
    BOOL a,b,c,d,e,f;
    if (dueDate == nil) {
        a = (_task.dueDate == nil?YES:NO);
    }
    else
        a = [dueDate isEqualToDate:_task.dueDate];
    if (reminderTime == nil) {
        b = (_task.reminderTime == nil?YES:NO);
    }
    else
        b = [reminderTime isEqualToDate:_task.reminderTime];
    c = (importance == _task.importance?YES:NO);
    d = (reccurentType == _task.reccurentType?YES:NO);
    e = [list isEqualToString:_task.parentID];
    f = [body isEqualToString:_task.body];
    if (a && b && c && d && e && f) {
        return NO ;
    }
    return YES;
}

    Вот такая радость в проекте..

    Запостил: Jackos, 15 Января 2014

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

    • ava Dummy00001 16.01.2014 03:50 # 0

      судя по строке №18, не говно.
      Ответить

      • ava TarasB 16.01.2014 10:16 # +1

        +1, в виде
        return !((dueDate == nil ? (_task.dueDate == nil?YES:NO) :  [dueDate isEqualToDate:_task.dueDate])
&& (reminderTime == nil ? (_task.reminderTime == nil?YES:NO) : [reminderTime isEqualToDate:_task.reminderTime])
&& (importance == _task.importance?YES:NO)
&& (reccurentType == _task.reccurentType?YES:NO)
&& [list isEqualToString:_task.parentID]
&& [body isEqualToString:_task.body])

        это бы смотрелось намного хуже
        Ответить

        • ava tirinox 16.01.2014 12:18 # +1

          А как же короткое замыкание &&
          Ответить

          • ava TarasB 16.01.2014 12:39 # 0

            Кстати да, семантика-то меняется.
            Ответить

            • ava Dummy00001 16.01.2014 14:03 # 0

              да. но есть шанс что компилер это соптимизирует аналогичным образом.
              Ответить

              • ava TarasB 16.01.2014 14:17 # +1

                Это смотря что там в этих методах
                Ответить

                • ava Dummy00001 16.01.2014 14:22 # +1

                  да. к сожалению.

                  эта проблема в особенности остра с капипищеным кодом. семантику понимаю, но хотелось бы что бы компиляторы какую опцию или модификатор для методов предоставляли, типа "в этой функции можно все нахер оптимить, ее просто какой-то пидар копи-пастом писал".
                  Ответить

      • ava roman-kashitsyn 16.01.2014 10:28 # 0

        Ещё бы переменным дать нормальные имена
        Ответить

        • ava krypt 16.01.2014 11:32 # 0

          И выкинуть бессмысленные тернарники
          Ответить

        • ava 1024-- 16.01.2014 12:22 # +1 

          BOOL p,a,c,k,e,r;
          Ответить

        • ava Dummy00001 16.01.2014 14:00 # 0

          посидеть пару часов поизобретать имена для переменных которые нужны только раз??
          Ответить

          • ava roman-kashitsyn 16.01.2014 14:08 # +1

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

            • ava Dummy00001 16.01.2014 14:16 # 0

              предствил себе как оно будет выглядить с "нормальными" именами

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

              ЗЫ "Совершенство достигнуто не тогда когде больше нечего добавить, а тогда когде больше нечего отнять." (с)
              Ответить

              • ava roman-kashitsyn 16.01.2014 14:23 # +1

                да банальных dueDateModified, reminderTimeModified, etc. вполне достаточно.

                Заставил таки себя дочитать код... подобный механизм проверки модификаций крайне неудобен и чреват проблемами. В зависимости от желаемых целей можно использовать контрольную сумму полей объекта или changeListener + dirty-флажок, выставляемый при обновлении полей субъектом.
                Ответить

    • ava bormand 16.01.2014 06:10 # 0

      А == там возвращает значения, эквивалентные YES и NO или нет?
      Ответить

      • ava krypt 16.01.2014 11:33 # 0

        Это Obj-C шная реализация классического говна a?true:false
        Просто у местного boolean так значения называются.
        Ответить

      • ava roman-kashitsyn 16.01.2014 11:39 # +2 

        [(x == y) applyNaturalConvertionToYesNo]
        Ответить

    • ava TarasB 16.01.2014 10:14 # +1

      какая-то странная запись if-else, какого хера в одной ветке есть скобки, в другой нет?
      Ответить

    • ava roman-kashitsyn 16.01.2014 14:29 # +1

      У нас в проекте был злобный Sonar, который считал больше трёх условий в одном if критической ошибкой. Приходилось вводить временные переменные, чтобы сделать сонар счастливее. При этом микрооптимизатор во мне стонал от боли, т.к. short-circuit семантика превращались в полное вычисление всех частей условия...
      Ответить

      • ava TarasB 16.01.2014 14:45 # +1

        Можно и без полных вычислений:
        if (a&&b || c&&d)
...

        заменяем на
        bool ab=false;
if (a)
  if(b)
    ab=true;
bool r=false;
if (ab)
  r=true;
else
{
  bool cd=false;
  if (c)
    if (d)
      cd=true;
}
if (r)
...

        И НИ ОДНОГО логического оператора.
        Ответить

        • ava roman-kashitsyn 16.01.2014 15:00 # +3

          A можно ввести ещё одну переменную
          bool changed = isAChanged();
changed = changed || isBChanged();
changed = changed || isCChanged();
          Ответить

        • ava TarasB 16.01.2014 20:48 # 0

          ой я всё равно облажался, во второй ветке cd заменяем на r
          Ответить

          • ava tirinox 16.01.2014 22:45 # +3

            Это говорит нам о том, что сей подход суть полное говно :)
            Ответить

        • ava wvxvw 17.01.2014 00:31 # +1 

          (defclass date () ())

(defclass start-date (date) ())

(defclass end-date (date) ())

(defparameter *standard-start-date* (make-instance 'start-date))

(defparameter *standard-end-date* (make-instance 'end-date))

(defparameter *standard-string* "standard")

(defgeneric was-changed (something &optional more-things-to-check)
  (:method ((this start-date) &optional more-things-to-check)
    (or (not (eql this *standard-start-date*))
        (destructuring-bind (end-date . some-string)
            more-things-to-check
          (was-changed end-date some-string))))
  (:method ((this end-date) &optional more-things-to-check)
    (or (not (eql this *standard-end-date*))
        (was-changed (car more-things-to-check))))
  (:method ((this string) &optional more-things-to-check)
    (declare (ignore more-things-to-check))
    (not (eql this *standard-string*))))

(was-changed *standard-start-date*
             (list (make-instance 'end-date) "non-standard"))
;; T
(was-changed *standard-start-date*
             (list *standard-end-date* *standard-string*))
;; NIL

          Настоящий ООП-стиль.
          Ответить

    • ava anonimb84a2f6fd141 18.01.2014 12:05 # 0

      В чем суть _task.importance?YES:NO ? Слабая типизация?
      Ответить

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