1. PHP / Говнокод #15609

    +152

    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
    public function validateItem() {
  $orderPricesSum = null;
  if($orderPricesSum == null) {
	$orderPricesSum = $this->getPricesSum();
  }

 if($this->minimal && $orderPricesSum < $this->minimal) {
	return false;
  }

  if($this->maximum && $orderPricesSum > $this->maximum) {
	return false;
  }

  return true;
}

    $orderPricesSum другой ведь какой-то может быть...

    Запостил: farit_slv, 28 Марта 2014

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

    • ava eandr67 28.03.2014 14:40 # +1

      Строчки 2-3 - полный бред.

      Несогласованные названия полей (либо minimal/maximal, либо minimum/maximum, но не вперемешку)

      Подозреваю, что getPricesSum - это обычный геттер. И тогда вся функция сводится к:

      return (empty($this->minimal)||$this->pricesSum>=$this->minimal)&&(empty($this->maximum)||$this->pricesSum<=$this->maximum);
      Ответить

      • ava Lowezar 28.03.2014 15:00 # +1

        Подозреваю, что всё-же оно что-то суммирует, для этого промежуточная переменная. Но сути не меняет, да. Хотя я бы не стал так сокращать, заменил бы только первые 4 строки на $orderPricesSum = $this->getPricesSum();. Развёрнутые ифы нагляднее демонстрируют назначение кода. Элементарная экономия времени при разборе чужого кода. :)
        Ответить

        • ava eandr67 28.03.2014 15:28 # 0

          С $orderPriceSum частично согласен. Но все же, если это геттер, то он должен суммировать только один раз и при последующих вызовах возвращать запомненную сумму.

          if, телом которого является "return false;", имеет смысл либо когда условие слишком сложное, либо когда имеется последовательность разнородных условий (т.е. проверяются разные сущности), либо имеется вложенность операторов и после этого if'а есть действия, отличные от retrn. Здесь же условие слишком простое, чтобы разбивать его на 2 if'а, да еще и вставлять после них "return true;".
          Ответить

      • ava farit_slv 28.03.2014 15:05 # 0

        Так я о том же, по сути $orderPricesSum это не глобальная какая то переменная, что бы ее переопределять и еще проверять на пустоту
        Ответить

        • ava Lowezar 28.03.2014 18:36 # 0

          Это, скорей всего, последствия рефакторинга.
          Ответить

    • ava gost 28.03.2014 15:08 # 0

      >>$orderPricesSum < $this->minimal
      >>$orderPricesSum > $this->maximum
      wut?
      Ответить

      • ava eandr67 28.03.2014 15:33 # 0

        Нет, не так. $this->minimal и $this->maximum могут быть "пустыми" ("", null, [], 0, false) и тогда сравнение с ними производить не нужно.
        Ответить

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