"PHP" / Говнокод #11502 Ссылка на оригинал

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
  23. 23
  24. 24
  25. 25
  26. 26
  27. 27
  28. 28
  29. 29
  30. 30
  31. 31
  32. 32
  33. 33
  34. 34
  35. 35
  36. 36
  37. 37
  38. 38
  39. 39
public function getAddressInfo($address) {
		if (!$address instanceof SalesOrderAddressShipping) return '';
		
		$country = $address->getCountry();
		$region = $address->getRegion();
		$city = $address->getCity();
		$zipCode = $address->getZipCode();
		$street = $address->getStreet();
		$houseNumber = $address->getHouseNumber();
		$building = $address->getBuilding();
		$appartment = $address->getAppartment();

		$addressInfo = '';
		$addressInfo .= $city . ', ' . $street . ' ' . $houseNumber;
		$addressInfo .= !empty($building) ? ', корп. ' . $building : '';
		$addressInfo .= !empty($appartment) ? ', кв. ' . $appartment : '';

		return $addressInfo;
	}

	public function getAddressInfoWithoutApartment($address) {
		if (!$address instanceof SalesOrderAddressShipping) return '';

		$country = $address->getCountry();
		$region = $address->getRegion();
		$city = $address->getCity();
		$zipCode = $address->getZipCode();
		$street = $address->getStreet();
		$houseNumber = $address->getHouseNumber();
		$building = $address->getBuilding();
		$appartment = $address->getAppartment();

		$addressInfo = '';
		$addressInfo .= $city . ', ' . $street . ' ' . $houseNumber;
		$addressInfo .= !empty($building) ? ', корп. ' . $building : '';
		//$addressInfo .= !empty($appartment) ? ', кв. ' . $appartment : '';

		return $addressInfo;
	}

а сам класс называется AdminOrderListContentView, ебиеговрот....

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

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

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

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

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

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

        Какой коллектив -- такой код.
        Ответить

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

          Дело даже не в том что можно было сформировать $addressInfo из свойств объекта $address не перекидывая их в промежуточные переменные (ну пожалуй кроме $address->getBuilding() и $address->getAppartment() которые гепотетически могут быть чем-то посложнее чем просто return $this->building). Говнокод скорее в копипасте без одной строки и даже в названии второго метода. Сразу видно что когда возникла необходимость усложнить задачу человек решил её незадумываясь тупо и в лоб.
          Ответить

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

            Да не дурите голову.
            Здесь всё ясно. Было спрошено с программиста, чтобы в вид возвращалась форматированная строка. Это и было сделано. Формотирование меняется -- меняется код. И что там будет послезавтра -- неизвестно. Вот и висят куски.

            Говнокод, говногод...
            Как маленький ребёнок, честное слово...

            Или, как говорится, тролль.
            Ответить

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

              Но позвольте, вы оправдываете существование двух функций с совершенно одиаковым кодом за разницей одной лиш строчки????
              Ответить

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

                Да.
                Две функции с близкими названиями совершают близкие действия.

                Если бы было две функции с непонятными и разными названиями, а они делали бы одно и то же -- это говноконструирование.
                Если бы было две функции с близкими названиями, а дела ли бы они совершенно разное -- говноконструирование.

                Если здесь есть говно- то это говноменеджмент, который привёл к тому, что из MVC выпал кусок. И этот выпавший кусок ещё достаточно прилично сделан!
                Гораздо ужаснее было бы, если бы этот кусок вообще был прямым текстом размещён посреди "бизнес-логики". Так он хоть упакован отдельно: с глаз долой, из сердца вон.

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

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

                    Предлагаю начать список с перечисления министерств и ведомств РФ =)
                    Ответить

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

                  Если кому-то интересно, как поступил бы истинный ценитель ProblemFactory: если форматировать адрес нужно часто и форматы немного отличаются в разных местах приложения, я бы написал AddressFormatter, который бы принимал строку наподобие printf. Часто употребляемые форматы сделал бы константами.
                  // %c - city, %s - street, %h - house %b - building
// %a - appartment, ?a{expr} - conditional
const ADDRESS_INFO = "%c, %s %h ?b{, корп. %b} ?a{, кв. %a}";
$displayAddr = Address\Formatter::format(ADDRESS_INFO, $address);
                  Ответить

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

                    Это ещё сложнее в использовании, чем просто сляпать куски по желанию.

                    Бессмысленно говорить "о хорошем коде", когда здесь дыра между разными людьми. Говно в человеческом факторе.

                    Форматер сработал бы, как, например, доп. тег в JSP. Но подобные коды, как выше, появляются именно от того, что кто-то не хочет делать форматирование, а кто-то не хочет его поддерживать.
                    Ответить

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

                    это хороший и полезный приём, надо всего то оценивать трудозатраты

                    представляю себе глаза проффессионнала-пхпшника, которому ради всего 2 вариантов форматирования, вместо 30 секунд работы "добавить уже сраный аргумент bool в эту сраную функцию", тимлид предложит написать небольшой такой парсер-форматтер
                    Ответить

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

                      да глаза будут удивленные. Ведь профи уже так и сделал еще вчера. пых развивается и с ним развиваются и прогеры.
                      Ответить

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

                      Если всего два формата - я бы тоже сделал флажок, даже не будучи профессионалом-похапешником. Будь их пять - я бы потратил 40 минут на написание форматтера.
                      Ответить

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

                        Как оно часто бывает оно сегодня два завтра три, через неделю уже все привыкли так делать.
                        Ответить

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

    if (!$address instanceof SalesOrderAddressShipping) return ''; поубивал бы !!! Исключение кинуть не бывает. Никакого понимания что делается и для чего, еще бы die в писали бы.
    Ответить

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

      Это как раз широко распространённый приём.
      Когда алгоритм устроен так, что наличие реального объекта не является обязательным условием, возвращают Null object -- дырку без поведения, в данном случае пустую строку.

      Это проблема не PHP и даже не конкретного программиста, данная проблема имеет более глубокие и исторические образующие причины.
      Ответить

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

      Зачем писать на php в стиле java, если можно писать на java в стиле php?
      Ответить

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

          Кстати, а в ПХП лучше сделали - специальный протокол, который (пытается) абстрагирует файловую систему, а jar'ы загружать / подкючать к проекту нужно в каждой оси по-разному.
          Ответить

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

            В java от системы к системе отличается только path separator. Подключается всё везде одинаково: 
            java -cp my.jar:other.jar:third.jar MainClass
            Тулы скрывают от вас эти различия.
            Ответить

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

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

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

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

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

            Vasiliy прав.
            Гораздо проще создать несколько динамических страничек на PHP, чем на java. Всё же, java для web в своей основе более сложное с инженерной точки зрения сооружение.
            Фреймворки тоже не помощники. Нужно ещё разбираться как они работают, как они устанавливаются.

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

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

    http://ideone.com/Ttoul 
    <?php
/* init in costructor */
function getCountry()    { return 'Country';     }
function getRegion()     { return 'Region';      }
function getCity()       { return 'City';        }
function getZipCode()    { return 'ZipCode';     }
function getStreet()     { return 'Street';      }
function getHouseNumber(){ return 'HouseNumber'; }
function getBuilding()   { return 'Building';    }
function getAppartment() { return 'Appartment';  }
 
/* init in costructor */
$func = array(
'country'     => array('getCountry',     'Customer address info: ' ),
'region'      => array('getRegion',      ', ' ),
'city'        => array('getCity',        ', ' ),
'zipCode'     => array('getZipCode',     ', ' ),
'street'      => array('getStreet',      ', ' ),
'houseNumber' => array('getHouseNumber', ', ' ),
'building'    => array('getBuilding',    ', корп. ' ),
'appartment'  => array('getAppartment',  ', кв. ' )
);
 
function getAddresInfo($cfg)
{
        global $func; 
        $CustomerAddrInfo = '';
 
        foreach ($cfg as $key=>$val) {
                if ($val == '1') 
                        $cfg[$key] = $func[$key][0]();
 
                $CustomerAddrInfo .= (!empty($cfg[$key]))? $func[$key][1] . $cfg[$key] : '';
        }
        return $CustomerAddrInfo;
}
 
/* for getAddresInfo($cfg) */
$addressInfo = array(
'country'     => '1',
'region'      => '1',
'city'        => '1',
'zipCode'     => '1',
'street'      => '1',
'houseNumber' => '1',
'building'    => '1',
'appartment'  => '1'
);
 
echo getAddresInfo($addressInfo);
$addressInfo['appartment'] = '0';
echo "\n";
echo getAddresInfo($addressInfo);
echo "\n";
$addressInfo['building'] = '0';
echo getAddresInfo($addressInfo);
?>
    Ответить

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

          Если где-то захотим отображать дом и корпус как 7/2, придётся переписывать движок. Как локальное решение может и сойдёт, но я бы так делать не стал.
          Ответить

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

            > Если где-то захотим
            < Все «хотелки» должны быть формализованы предварительно, желательно.

            Вопрос сводиться к написанию автоматически конфигурируемого парсера выхлопа.
            Без спецификации формат выхлопа - это не серьёзный разговор.
            Кстати говоря. Вместо '1' в $addressInfo могут содержаться индексы формата.
            т. е. если не ноль, то $func[$key][$val]
            Ответить

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

Где здесь C++, guest?!

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

    ↑
    8
    ↓