=()= / Говнокод #23249 Ссылка на оригинал

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
  21. 21
  22. 22
if ( $MonitorMode eq \">=\" )
{
  if ( $NbrProcesses < $ProcNumber )
  {
    $Rule->Status(TRUE);
  }
}
elsif ( $MonitorMode eq \"<=\" )
{
  if ( $NbrProcesses > $ProcNumber )
  {
    $Rule->Status(TRUE);
  }
}
else
{
  if ( $NbrProcesses != $ProcNumber )
  {
    $Session->Value(\"PROCESSMODE\", \"\" );
    $Rule->Status(TRUE);
  }
};

Кровавый ентерпрайз. Кусок кода мейд бай ХулетПакард

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

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

  • в оригинале код был в двойных ковычках заквотен?

    нормальный код. просто и понятно. по нему можно учится как из мух слонов не делать. но я догадываюсь что из простого и понятного большой карьеры не сделаешь, почему этот код и говно. тем более, если его каждый индус может понять и - еще хуже - поменять, то карьера закончится еще скорее. быстрее апгрейдим все на энтерпрайз! https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition
    Ответить
    • Этот код говно потому, что простой и понятный if->elsif->elsif превратили в четыре if'a, один elsif и один else до кучи увеличив цикломатическую сложность на пустом месте.

      Да, в кавычках - оно часть конфигурационного файла лежащего в БД
      Ответить
      • > что простой и понятный if->elsif->elsif превратили в четыре if'a

        э? ты про что?

        $MonitorMode это очевидно состояние, а как во всех нормальных fsm первая проверка, это проверка состояния.

        > цикломатическую сложность

        каким именно из 73 мне известных определений вы пользуетесь? поройтесь немного, найдете себе определение маккэйба по которому этот код будет просто само совершенство.
        Ответить
        • Я пользуюсь классическим определением про граф потока исполнения (собственно предложенное Маккейбом) 🙂

          У исходного кода она, грубо, 16, а при уходе от внутренних if'ов - уже 6.

          Ну и на счет состояния можно с тем же успехом утверждать, что состояние - это $ProcNumber, а $MonitorMode- это модификатор влияющий на способ проверки этого состояния
          Ответить
          • > Я пользуюсь классическим определением про граф потока исполнения 🙂

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

            с другой стороны, ты тогда должен радоватся что тут не switch/case. иначе бы сложность была 123123123123123, плюс/минус 1.
            Ответить
            • Да ну ладно, эдак можно долго переливать из пустого в порожнее. Вот вам убийственный аргумент - лично для меня
              вот такой вариант на порядок читабельнее и способствует пониманию происходящего при беглом просмотре, без спотыкания на ненужной лестнице 🙂

              if ( ($MonitorMode eq ">=") && ($NbrProcesses < $ProcNumber) )
              {
                  $Rule->Status(TRUE);
              }
              elsif ( ($MonitorMode eq "<=") && ($NbrProcesses > $ProcNumber)  )
              {
                  $Rule->Status(TRUE);
              }
              elsif($NbrProcesses != $ProcNumber)
              {
                  $Session->Value(\"PROCESSMODE\", \"\" );
                  $Rule->Status(TRUE);
              };
              Ответить
              • Я тут наконец подумал и понял, что этот код неправильный (как минимум, не эквивалентен тому, что в топике).
                В этом варианте статус выставляется во всех ветках. В топике если мод >=, но условие на число процессов не выполнено, ничего не произойдёт, т.к. альтернативные ветки не будут просматриваться.
                Ответить
                • Это почему же?

                  ($MonitorMode eq ">=") && ($NbrProcesses < $ProcNumber) - это одно логическое выражение, атомарное относительно if. Если оно ЦЕЛИКОМ не выполняется, то управление передается следующему elsif
                  Ответить
                  • > Это почему же?
                    > Если оно ЦЕЛИКОМ не выполняется, то управление передается следующему elsif

                    Вот именно поэтому.
                    Давай проверим на конкретных значениях. Допустим, выполнены условия:
                    $MonitorMode = ">="
                    $NbrProcesses > $ProcNumber

                    В коде из топика мы войдём в первую ветку (if ( $MonitorMode eq \">=\" )), проверим условие ( $NbrProcesses < $ProcNumber ), сочтём его ложным и на этом успокоимся. Остальные условия проверяться не будут, потому что самый первый if уже сработал.

                    В твоём коде мы получим false в первой и во второй ветках, в итоге сработает последняя и выставит Status(TRUE).

                    Видишь, код не эквивалентный.
                    Ответить
                    • Позор на мои седины...

                      И ведь хотел другой вариант предложить изначально, но, так как давно с перлом не общался, решил не выделываться 🙂

                      AS учитывая, что там в коде сначала по умолчанию FALSE устанавливается

                      if ( $MonitorMode eq ">=" )
                      {
                          $Rule->Status($NbrProcesses < $ProcNumber);
                      }
                      elsif ($MonitorMode eq "<=" )
                      {
                          $Rule->Status($NbrProcesses > $ProcNumber);
                      }
                      else
                      {
                          $Session->Value(\"PROCESSMODE\", \"\" );
                          $Rule->Status($NbrProcesses != $ProcNumber);
                      };
                      Ответить
                    • Ну вот, кстати, если логику всего скрипта посмотреть, то последний eslif должен так выглядеть:
                      elsif(($MonitorMode eq "==") && ($NbrProcesses != $ProcNumber))

                      Тогда оно и работать корректно будет и определяем условие явно, что тоже плюс
                      Ответить
                      • А можно не трогать нормальный код неумелыми ручонками, и тоже все работать будет.
                        Ответить
                        • Можно, но они зачем то потрогали, потому и работает через жопу и приходится лезть внутрь и разбираться.
                          Ответить
                      • > ($MonitorMode eq "==")
                        А вот это годный фикс. В оригинальном коде он бы тоже не помешал. А то сиди и думай, что там в else обрабатывается - "==" или "!=".
                        Ответить
    • if ( $NbrProcesses != $ProcNumber ) {
        $Rule->Status(TRUE);
        if ( ($MonitorMode eq ">=") and ($NbrProcesses >= $ProcNumber) or
             ($MonitorMode eq "<=") and ($NbrProcesses <= $ProcNumber) ) {
          $Session->Value("PROCESSMODE", "" );
        }
      }
      Ответить
      • говно. ты fsm сломал. в фсм первым ты проверяшь состояние ($MonitorMode), и в зависимости от состояния, происходят дальнейшие действия.

        ЗЫ ты and/or думаю перепутал. ты хотел &&/||. но вроде у тебя тут это к багу не ведет.
        Ответить
          • если не как я фсмами увлекаешься, то простимо. и может это я затупил: может у них совсем не фсм? - а $MonitorMode это tied переменная которая при каждом обращении, своп в память зачитывает? или это вообще perl6, и if/elsif/else переопределены, и на самом деле этот код посылает сигнал тостеру новый сандвич сделать? хез.
            Ответить
            • да просто небось плагин какой для nagios, который условия для алармов считает. Я просто поторопился, невнимательно код прочитал, этот вариант более адекватный http://govnokod.ru/23249#comment388949
              Ответить
              • "упростить" примитивный код очень сложно 😉

                оригинальный код подоходит к правилам "явного программирования" (которые никто никогда конкретно не сформулировал). это когда у тебя на одну строчку одно выражение, и нет никаких/минимальные побочные эффекты.
                Ответить
  • синь прыщеблядь соснула глаза
    Ответить
    • Там, где капустные грядки
      Красной водой поливает восход,
      Кленёночек маленький матке
      Зелёное вымя сосёт.
      Ответить
        • И невольно в море хлеба
          Рвётся образ с языка:
          Отелившееся небо
          Лижет красного телка.
          Ответить
        • В 47 хромосоме прописано

          Из западного ануса все жрёте вы говно,
          За кока-колу сраную вы продались давно.
          Подстилки подпиндосные, дадим вам пососать,
          И знать тогда вы будете, как анус им лизать.

          Сосётся как? Свой анус не порвали?
          Говно всё жрёте? В жопу вас ебали?
          Легли под Запад; в жопу им даёте,
          Подстилки. Анус лижете, сосёте.


          Квинтессенция "русского мира", если вдуматься,
          ничего правдивее, точнее, пронзительнее на русском
          языке не написали, и уже не напишут. Лавров и
          Маяковский с Пушкиным в одном флаконе.
          Ответить
  • Говно код в том, что

    1. Проверяется один режим, а проверка, по-факту, выполняется другая. Для ясности перевернул второе условие.

    $MonitorMode eq ">=" (больше или равно)
    $ProcNumber > $NbrProcesses (строго больше)


    $MonitorMode eq "<=" (меньше либо равно)
    $ProcNumber < $NbrProcesses (строго меньше)


    2. Совершенно не ясно, являются ли эти случаи попарно равнозначными

    $MonitorMode eq "==" && $ProcNumber == $NbrProcesses
    $MonitorMode eq "!=" && $ProcNumber == $NbrProcesses


    $MonitorMode eq "==" && $ProcNumber != $NbrProcesses
    $MonitorMode eq "!=" && $ProcNumber != $NbrProcesses
    Ответить
  • В предположении, что "написанному верить" и всё так как и задумано можно сделать так:

    my %check_proc_number = (
    	">=" => [ 0, sub { $NbrProcesses < $ProcNumber; } ],
    	"<=" => [ 0, sub { $NbrProcesses > $ProcNumber; } ],
    );
    my @check_proc_number =
    		( 1, sub { $NbrProcesses != $ProcNumber; } );
    
    my ( $set_proc_mode, $check_proc_number ) =
    	$check_proc_number{$MonitorMode} // @check_proc_number;
    
    $set_proc_mode and $Session->Value("PROCESSMODE", "" );
    $check_proc_number->() and $Rule->Status(TRUE);
    Ответить

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

Помни, guest, за тобой могут следить!

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


    8