Why is this shitcode?

Question

WordPress

The more newbies you despise and ignore, the more shit code you'll have to work with on side projects

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>

Subject. I want to understand my mistakes and not repeat them again.
For now I'm done on "Works, oh well"

Answer 1

if(is_singular( 'portfolio' ) ) {
This is the so-called WordPress codestyle (arranging brackets and spaces) - forget it like a bad dream, phew.
And if you do follow, then follow everywhere, but here you have it this way, here it’s different.
<?
You can not use a short opening construction, write in full.
Exception - <?=- I would even recommend using such a notation if you really don't need anything other than the output.
Code mixed with markup is very bad and hard to read. WordPress, unfortunately, does not offer any standard options to avoid this, either it seems to them that this is the norm, or the size of the code base is such that there is no way to rewrite it normally. Come up with a separation option for yourself from a simple removal of logic in a function (for example, in the last block of code, invent some kind of getPostTags function and get markup for it) to template engines.
The indentation is extremely strange - it is inconvenient to read, in addition, the folding code will break in Sublime for example (he uses indentation for this). Write code on the same nesting where the tag was opened.
Too lazy to delve into the logic. Take a look yourself again and evaluate whether there are repeating pieces of code, whether it can be done more efficiently...
As you can see, most of the jambs relate to visual formatting - and this is logical, because we read the code much more often than we write it, and we need to make this process convenient.
Bring formatting to a single form - and it will be fine.
Shit, not shit - I rarely have the thought "wow, great code!".
If the code can be read without problems and understand what the author wanted, it is normal, and that's okay.

Answer 2

At least there is no formatting and all the noodles are in one function - it turns out to be hard-to-read non-logical garbage.
Break it down into functions. Remove the repetitions in the function and pass the differences to it. Set up automatic formatting on save at least.
And make it a rule to turn on the debug mode - I'm sure there are notes.