Участник:ArmorAdmin/Говнокод — различия между версиями

Версия 19:24, 25 августа 2010

Автор(ы): Чобиток Василий

Пример № 1 (PHP 5)

14.08.2010

Задача: известно, что путь к картинке хранится в переменной $image_path. Картинка может быть в одном из форматов, который поддерживается браузерами (JPEG, GIF и т. п.). Необходимо картинку передавать в браузер не прямой ссылкой, а при обращении к скрипту.

Естественно, что при получении картинки скриптом в начале PHP-скрипт должен сообщить в заголовке тип передаваемых данных, например: «Content-type: image/jpeg».

Программист решает эту задачу следующим образом:

$extension = strtolower(strrchr(basename($image_path), '.'));

switch($extension) {
  case '.jpg': header('Content-type: image/jpeg'); break;
  case '.jpeg': header('Content-type: image/jpeg'); break;
  case '.png': header('Content-type: image/png'); break;
  case '.gif': header('Content-type: image/gif'); break;
}

В этом коде проблемы следующие:

1. Привязка к расширению. Не всегда расширение соответствует реальному формату данных. Дай файлу в формате JPEG расширение «.gif» и этот код перестанет работать. Впрочем, это некритичная проблема, т.к. любой сайтовладелец, должен следить за адекватностью контента и данная проблема может никогда и не проявится.
2. При появлении новых форматов изображений (SVG набирает силу) придётся менять код внутри структурного оператора switch и добавлять новые операторы.

Решение проблемы: в PHP есть функции, которые возвращают тип изображения по сигнатуре первых байтов его файла (exif_imagetype), а также Mime-тип изображения по его типу. Рефакторинг предыдущего решения даёт следующий результат:

$imgType = exif_imagetype($image_path);
header('Content-type: ' . image_type_to_mime_type($imgType));

Здесь картинки работают независимо от расширения, нет привязки к ограниченному списку форматов, размер кода сократился в три раза.

Пример № 2 (PHP 5)

16.08.2010

Этот пример идет в продолжение предыдущего.

Задача: известен тип ($imgType) изображения, которое хранится по известному пути ($image_path). Необходимо загрузить изображение в память (переменная $img) для последующей программной обработки.

Самый простой способ сделать это следующим образом:

switch ($imgType) {
  case IMAGETYPE_GIF:
    $img = imagecreatefromgif($image_path); break;
  case IMAGETYPE_JPEG:
    $img = imagecreatefromjpeg($image_path); break;
  case IMAGETYPE_PNG:
    $img = imagecreatefrompng($image_path); break;
}

Мы простых путей не ищем и делаем то же самое так:

$imgCreateMethods = array (
    IMAGETYPE_GIF => 'imagecreatefromgif',
    IMAGETYPE_JPEG => 'imagecreatefromjpeg',
    IMAGETYPE_PNG => 'imagecreatefrompng',
);

$img = $imgCreateMethods[$imgType]($image_path);

Хоть кода и меньше, но первый вариант на мой взгляд читается проще. Так зачем в этом случае городить огород?

Суть решения в том, что, как говорилось в предыдущем примере, заранее может быть неизвестен полный перечень форматов, что является изменчивой частью системы. Как утверждают классики, изменчивая часть должна быть инкапсулирована (сокрыта) от основной системы. Поэтому перечень имён функций для создания изображения в памяти вынесен в отдельный массив $imgCreateMethods. Этот массив может быть объявлен во включаемом остальными файлами файле настройки (например, config.php).

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

Пример № 3 (SQL, Interbase)

25.08.2010

Индусы нервно курят в сторонке. Этот пример SQL-кода отечественного производителя я привожу знакомым в качестве вершины говнокодерского искусства. Ниже дан пример хранимой процедуры в базе данных Interbase. Процедура приведена в сокращенном на несколько сотен строчек виде (см. соответствующие комментарии).

CREATE PROCEDURE STATISTIC_CARD_ZERO_ONE (
    causeid integer,
    causeorgid integer)
as
declare variable acauseid integer;
declare variable aorgid integer;
declare variable arequested double precision;
declare variable arequestcurrencyid integer;
/* 
* Здесь было объявление более 100 переменных,
* которые условно не показаны
*/
declare variable P3_381 date;
begin
  for
    select
      CAUSEID,ORGID,REQUESTED,REQUESTCURRENCYID,
/*... более 100 полей условно не показаны ...*/
      P3_36,P3_37,P3_38,P3_381
    from
      STATISTIC
    where
      CAUSEID = :CAUSEID and ORGID = :CAUSEORGID
    into
      :ACAUSEID,:AORGID,:AREQUESTED,:AREQUESTCURRENCYID,
/*... более 100 переменных условно не показаны ...*/
      :AP3_36,:AP3_37,:AP3_38,:AP3_381
  do
    begin
      if (AREQUESTED is null or AREQUESTED <> 0) then   
        begin
           update STATISTIC set
             REQUESTED = 0
           where
             CAUSEID = :ACAUSEID and ORGID = :AORGID;
        end       
      if (AREQUESTCURRENCYID is null or AREQUESTCURRENCYID <> 3) then   
        begin
           update STATISTIC set
             REQUESTCURRENCYID = 3
           where
             CAUSEID = :ACAUSEID and ORGID = :AORGID;
        end

/* несколько десятков аналогичных блоков if...then не показаны */
		
      if (AP1_84 is null or AP1_84 <> 'F') then   
        begin
           update STATISTIC set
             P1_84 = 'F'
           where
             CAUSEID = :ACAUSEID and ORGID = :AORGID;
        end  

/* несколько десятков аналогичных блоков if...then не показаны */
		
      if (AJOINED <> '') then   
        begin
           update STATISTIC set
             JOINED = ''
           where
             CAUSEID = :ACAUSEID and ORGID = :AORGID;
        end   

/* несколько десятков аналогичных блоков if...then не показаны */

      if (AP3_381 is not null) then   
        begin
          update STATISTIC set
            P3_381 = CAST(null as DATE)
          where
            CAUSEID = :ACAUSEID and ORGID = :AORGID;
        end            
    end      
end

Если не испугаться объема процедуры, то понять смысл в ней происходящего несложно. В имеющейся записи «обнуляются» значения полей, им присваиваются значения по умолчанию. Но как это происходит?! Это происходит вызовом инструкции UPDATE для каждого поля в отдельности (а их — сотни)!!!

Более того, полей в таблице автора сего шедевра настолько много, что со своей процедурой он не влазил в имевшиеся ограничения на объём хранимых процедур, и таких процедур у него несколько. Но тут, тем не менее, стоит отдать должное нашему трудолюбивому кодеру, он написал не обычный говнокод, а оптимизированный говнокод — UPDATE у него вызывается всё же не для всех полей, а только для тех, чьи значения отличаются от значений по умолчанию :-)

Для меня до сих пор остаётся загадкой, почему автору этого творения не пришёл в голову самый простой из возможных вариантов?

CREATE PROCEDURE STATISTIC_CARD_ZERO_ONE (
    causeid integer,
    causeorgid integer)
as
begin
  update STATISTIC
  set
    REQUESTED = 0,
    REQUESTCURRENCYID = 3,
    P1_84 = 'F',
/*... более 100 полей условно не показаны ...*/
    
  where
    CAUSEID = :CAUSEID and ORGID = :CAUSEORGID
end

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

С нормально объявленными для полей значениями по-умолчанию всё это хозяйство сводится к двум строчкам кода:

delete from STATISTIC where CAUSEID = :CAUSEID and ORGID = :CAUSEORGID;
insert into STATISTIC (CAUSEID, ORGID) values (:CAUSEID, :CAUSEORGID);

Трудолюбивый дурак такое же зло, как и дурак с инициативой. Если человек не привык работать задницей, отсиживая её часами копируя строки кода, и у него есть хоть немного лени, то ваяя подобное остановится и подумает: «А не дурак ли я?»