Почему это говнокод?

Question

[godsplane]

Чем больше новичков вы презираете и игнорируете тем с большем количеством говнокода вам придется работать в сторонних проектах

spoiler

<div class="portfolio-container-tags">
 <div class="portfolio-container-tags">
  <?php
      $tags = get_the_tags();
      if(is_singular( 'portfolio' ) ) {
         $tag = array_pop($tags);
         $numberPost = 1000;
      }
      elseif (is_singular('services')){
        $tag = array_pop($tags);
        $numberPost = 1000;
      } else {
        $numberPost = 6;
      }
      if (is_tag()) {
        $tag = get_queried_object();  
        $numberPost = 1000;
      }
      $video = get_posts( array(
        'numberposts' => $numberPost,
        'tag' => $tag->slug,
        'orderby'     => 'menu_order',
        'order'       => 'DESC',
        'post_type'   => 'video'
      ) );
  ?>
  <div class="video-wrapper">
    <?php
      if ($video) {
    ?>
    <h2>Видеообзор
      <?php
     echo mb_strtolower($tag->name)
    ?>
    </h2>
    <?
      }
    ?>
    <div class="row">
      <?php
        foreach( $video as $post ){ setup_postdata($post);
	    ?>
      <div class="portfolio-item video-item">
        <div class="video-content">
          <?php
            the_content();
          ?>
        </div>
        <h2>
          <?php
            the_title();
          ?>
        </h2>
      </div>
      <?php 
        }
        wp_reset_postdata();
      ?>
    </div>
  </div>
  <div class="portfolio-wrapper row container">
    <?php 
        $tags = get_the_tags();
        if(is_singular( 'portfolio' ) ) {
        $tag = array_pop($tags);
        $numberPost = 1000;
        }
        else if (is_singular('services')){
          $tag = array_pop($tags);
        $numberPost = 6;
        } else {
          $numberPost = 6;
        }
        if (is_tag()) {
          $tag = get_queried_object();  
          $numberPost = 1000;
        }
        $lastposts = get_posts( array(
        'numberposts' => $numberPost,
        'tag' => $tag->slug,
        'orderby'     => 'menu_order',
        'order'       => 'DESC',
        'post_type'   => 'portfolio'
        ) );

      foreach( $lastposts as $post ){ setup_postdata($post);
	 ?>
    <div class="portfolio-item">
      <div class="portfolio-images">
        <a class="fancybox" rel="group" href="<?php the_post_thumbnail_url('large')?>">
          <img src="<?php the_post_thumbnail_url()?>" alt="Фотография">
        </a>
      </div>
      <h2>
      <?php
        the_title();
      ?>
      </h2>
      <?php
        the_content();
      ?>
    </div>
    <?php 
 }
 wp_reset_postdata();
 ?>
  </div>
  <?php
      if(!is_singular('services')) {
        $tags = get_tags();
        $html = '<div class="post_tags"><a href="/portfolio/" class="tags-name">Все</a>';
        foreach ( $tags as $tag ) {
          $tag_link = get_tag_link( $tag->term_id );
          $html .= "<a href='{$tag_link}' title='{$tag->name}' class='tags-name {$tag->slug}'>";
          $html .= "{$tag->name}</a>";
        }
        $html .= '</div>';
        echo $html;
      }
  ?>
</div>


</div>

Сабж. Я хочу понять свои ошибки и больше их не повторять.
На данный момент я сделал на "Работает, да и ладно"

Comments

[Saboteur]

ну, например, у вас беда с отступами, читать очень неприятно..

[Дмитрий]

1 форматирование
2 где стандарты написания кода? https://make.wordpress.org/core/handbook/best-prac...
3 говнокод:
например, $tag - сколько раз перееопределяется? и почему в него кладутся разные вещи? или, сколько раз есть $numberPost = 1000;?
дублирование кода
потенциальные проблемы с производительностью ( is_tag() + $numberPost = 1000; + кастомное WP_Query - зачем? можно ведь было отфильтровать основную query для is_tag() тип страниц. Так поулчается лишний запрос)
и тп

чем править этот код, его проще переписать

Answer 1

[Максим Морев]

if(is_singular( 'portfolio' ) ) {
Это так называемый WordPress codestyle (расставление скобок и пробелов) - забудьте как страшный сон, фу.
А если уж и следуете - то следуйте везде, а у вас тут так, тут иначе.

<?
Нельзя короткую открывающую конструкцию использовать, пишите полностью.
Исключение - <?= - я бы даже рекомендовал использовать такую запись, если кроме вывода действительно ничего не надо.

Код вперемешку с разметкой - это очень плохо и сложно для восприятия. WordPress, к сожалению, не предлагает никаких штатных опций этого избежать, им то ли кажется, что это норма, то ли объём кодовой базы такой, что переписать нормально возможности нет. Сами себе придумайте вариант разделения от простого выноса логики в функции (например, в последнем блоке кода выдумайте какую-нибудь функцию getPostTags и ей получайте разметку) до шаблонизаторов.

Отступы крайне странные - читать неудобно, кроме того, код фолдинг сломается в Саблайме например (он использует отступы для этого). Пишите код на той же вложенности, где тег открыли.

В логику вникать лень. Сами посмотрите ещё раз и оцените, есть ли там повторяющиеся куски кода, можно ли сделать эффективнее...

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

Answer 2

[Вова Дружаев]

Как минимум форматирования нет и вся лапша в одной функции - получается сложночитаемая не логическая фигня.
Разбей на функции. Убери повторы в функцию и передавай в неё различия. Настрой автоматическое форматирование по сохранению хотя бы.

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

Comments

[godsplane]

С форматирование php in html у меня не справляется плагин для vscode увы.

[Дмитрий]

godsplane, поставь phpcs