Govnokodu.Net

Пример рефакторинга «извлечение функции» методом составления плана

Исходный код

function custom_print ($custom_category, $custom_template, $aviable, $custom_from, $custom_limit, $custom_cache, $do){
  global $db, $is_logged, $member_id, $xf_inited, $cat_info, $config, $user_group, $category_id, $_TIME, $lang;

  $do = $do ? $do : "main";
  $aviable = explode ('|', $aviable);

  if(!(in_array($do, $aviable)) AND ($aviable[0] != "global")) return "";

  $custom_category  = $db->safesql(str_replace(',', '|', $custom_category));
  $custom_from = intval($custom_from);
  $custom_limit = intval($custom_limit);
  $thisdate = date ("Y-m-d H:i:s", (time()+ $config['date_adjust']*60));

  if (intval($config['no_date'])) $where_date = " AND date < '".$thisdate."'"; else $where_date = "";

  $tpl = new dle_template;
  $tpl->dir = TEMPLATE_DIR;

  //if ($custom_cache == "yes") $config['allow_cache'] = "yes"; else $config['allow_cache'] = false;
  if ($is_logged AND ($user_group[$member_id['user_group']]['allow_edit'] AND !$user_group[$member_id['user_group']]['allow_all_edit'])) $config['allow_cache'] = false;

  $content = dle_cache("custom", "cat_".$custom_category."template_".$custom_template."from_".$custom_from."limit_".$custom_limit, true);

  if ($content) { return $content; }
  else {

    $allow_list = explode (',', $user_group[$member_id['user_group']]['allow_cats']);

    if ($allow_list[0] != "all") {
      if ($config['allow_multi_category']) {
        $stop_list = "category regexp '[[:<:]](".implode ('|', $allow_list).")[[:>:]]' AND ";
      } else {
        $stop_list = "category IN ('".implode ("','", $allow_list)."') AND ";
      }
    } else $stop_list = "";

    if ($user_group[$member_id['user_group']]['allow_short']) $stop_list = "";

    if ($cat_info[$custom_category]['news_sort'] != "") $config['news_sort'] = $cat_info[$custom_category]['news_sort'];
    if ($cat_info[$custom_category]['news_msort'] != "") $config['news_msort'] = $cat_info[$custom_category]['news_msort'];

    if ($config['allow_multi_category']) {
        $where_category = "category regexp '[[:<:]](".$custom_category.")[[:>:]]'";
    } else {
        $custom_category = str_replace ("|", "','", $custom_category);
        $where_category = "category IN ('".$custom_category."')";
    }

    $sql_select = "SELECT " . PREFIX . "_post.id, gallery, autor, date," . PREFIX . "_post.image," . PREFIX . "_post.imgtype, short_story, full_story, " . PREFIX . "_post.xfields, title, category, alt_name, " . PREFIX . "_post.comm_num, " . PREFIX . "_post.allow_comm, allow_rate, " . PREFIX . "_post.rating, " . PREFIX . "_post.vote_num, news_read, " . PREFIX . "_post.flag, " . PREFIX . "_users.fullname FROM " . PREFIX . "_post , " . PREFIX . "_users WHERE " . PREFIX . "_post.autor=" . PREFIX . "_users.name and ".$stop_list.$where_category." AND approve = '1'".$where_date." ORDER BY ".$config['news_sort']." ".$config['news_msort']." LIMIT ".$custom_from.",".$custom_limit;

    include (ENGINE_DIR.'/modules/show.custom.php');

    if ($config['files_allow'] == "yes")
      if ( strpos( $tpl->result['content'], "[attachment=" ) !== false)
      {
        $tpl->result['content'] = show_attach($tpl->result['content'], $attachments);
      }

    create_cache("custom", $tpl->result['content'], "cat_".$custom_category."template_".$custom_template."from_".$custom_from."limit_".$custom_limit, true);
  }
  return $tpl->result['content'];
}

Что не так в этом коде

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

Я начну извлечение, а желающие могут продолжить.

Вариант рефакторинга исходного кода

function is_aviable($aviable, $do)
{
  $do = $do ? $do : "main";
  $aviable = explode ('|', $aviable);

  if (!(in_array($do, $aviable)) AND ($aviable[0] != "global")) returnfalse;
  return true;
}

function custom_print($custom_category, $custom_template, $custom_from, $custom_limit)
{
  $cache_key = custom_print_cache_key($custom_category, $custom_template, $custom_from, $custom_limit);
  $content = dle_cache("custom", $cache_key, true);

  if ($content) return$content;
  
  $content = custom_print_content($custom_category, $custom_template, $custom_from, $custom_limit);

  create_cache("custom", $content, $cache_key, true);
  return $content;
}

Первым шагом я выкинул за пределы исходной функции всю работу с параметрами $aviable и $do: они касаются, видимо, проверки прав доступа, и не имеют никакого отношения собственно к контенту.

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

Фактически, на верхнем уровне осталось только кеширование, а рендеринг ушёл куда-то вглубь, что правильно — теперь у меня есть простой способ добраться до контента в обход кеширования.

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

Дальше я точно так же максимально обобщённо реализую рендеринг контента.

function custom_print_content($custom_category, $custom_template, $custom_from, $custom_limit)
{
  global $db;
  $sql_select = custom_print_prepare_sql($custom_category, $custom_template, $custom_from, $custom_limit);
  include (ENGINE_DIR.'/modules/show.custom.php');

  $content = $tpl->result['content'];
  $content = custom_print_handle_attachments($content, $attachments);

  return $content;
}

function custom_print_handle_attachments($content, $attachments)
{
  global $config;

  if ($config['files_allow'] == "yes")
    if (strpos( $tpl->result['content'], "[attachment=" ) !== false)
      return show_attach($content, $attachments);

  return $content;
}

Эти две новых функции теперь тоже максимально сконцентрированы на своих целях: одна рендерит контент в целом, другая — обрабатывает вложения. Ни одна, ни другая ничего не знает об особенностях работы друг друга. Важно лишь что на входе и что на выходе.

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

Функция для формирования SQL-запроса будет совсем простой:

const CUSTOM_PRINT_FIELDS = PREFIX . "_post.id, gallery, autor, date," . PREFIX . "_post.image," . 
                            PREFIX . "_post.imgtype, short_story, full_story, " . 
                            PREFIX . "_post.xfields, title, category, alt_name, " . 
                            PREFIX . "_post.comm_num, " . PREFIX . "_post.allow_comm, allow_rate, " . 
                            PREFIX . "_post.rating, " . PREFIX . "_post.vote_num, news_read, " . 
                            PREFIX . "_post.flag, " . PREFIX . "_users.fullname";
const CUSTOM_PRINT_FROM = PREFIX . "_post , " . PREFIX . "_users";

function custom_print_prepare_sql($custom_category, $custom_template, $custom_from, $custom_limit)
{
  global $config;
  $where_conditions = custom_print_where_conditions($custom_category, $custom_template);
  return "SELECT " . CUSTOM_PRINT_FIELDS . " FROM " . CUSTOM_PRINT_FROM . 
    " WHERE " . implode(' and ', $where_conditions) .
    " ORDER BY ".$config['news_sort']." ".$config['news_msort'].
    " LIMIT ".intval($custom_from).",".intval($custom_limit);
}

Дальше предлагаю продолжить самостоятельно: осталось реализовать формирование набора условий для запроса (функция должна возвращать массив выражений, которые будут соединены по И), ну и про функцию получения ключа кеша нужно не забыть.

Теория

Сергей Галанин