0
\$\begingroup\$

I am job-searching and putting together some code samples. Just wondering how it looks.

Here is an example of some code, from a custom WordPress theme, that works and is in production:

Homepage Template

<?php
/*
Template Name: Homepage
Requirements: Advanced Custom Fields plugin
*/
?>


<?php get_header(); ?>

<section class="hero">

    <div class="container">
        <main>
            <p class="h1">
                <?php the_field('hero_content'); ?>
            </p>
        </main>
    </div>

    <div class="testimonials">
        <ul class="slider">
            <?php get_template_part('partials/testimonials'); ?>
        </ul>
    </div>

</section>

<main id="main">

  <?php the_content(); ?>

  <div class="details">

        <div class="heading">
            <h1><?php _e('What We Build','ssae'); ?></h1>
        </div>

        <?php get_template_part('partials/details'); ?>

  </div>

    <div class="stats">

        <?php 
            while ( have_rows('stats') ) : the_row();
            $label = get_sub_field('label');
            $value = get_sub_field('value');
        ?>

            <div class="stat">
                <div class="label"><?php echo $label; ?></div>
                <div class="value"><?php echo $value; ?></div>
            </div>

        <?php
            endwhile;
        ?>

    </div>

</main>

<?php get_template_part('partials/callout'); ?>

<?php get_template_part('partials/featured-cs'); ?>

<?php get_footer(); ?>

Partials

callout

<?php
/* Callout, links to contact page. */
?>

<section class="callout"><a href="<?php echo site_url('/contact/'); ?>">
    <div class="container">
        <div class="text">
            <span class="icon-calendar solo"></span>
            <h2 class="h1"><?php _e('Schedule a Discussion','ssae'); ?></h2>
        </div>
    </div>
</a></section>

details

<?php
/* Display a title, block of text, and icon for each item. Items pulled from a repeater field in the backend. */
?>

<?php 
    while ( have_rows('details') ) : the_row();
    $title = get_sub_field('title');
    $icon_class = get_sub_field('icon_class');
    $text = get_sub_field('text');
?>

    <div class="detail">
        <span class="icon-<?php echo $icon_class; ?> solo"></span>
        <h2><?php echo $title; ?></h2>
        <?php echo $text; ?>
    </div>

<?php
    endwhile;
?>

testimonials

<?php
/* Display the 3 Most Recent Testimonials (a custom post type) */
?>

<?php 
    $loop = new WP_Query( array( 
        'post_type' => 'testimonials',
        'posts_per_page' => 3
    ) ); 
    while ( $loop->have_posts() ) : $loop->the_post(); 
?>
    <?php
    $thumb_id = get_post_thumbnail_id($post->id);
    $thumbnail = wp_get_attachment_image_src( $thumb_id, 'thumbnail' );
    $thumb_alt = get_post_meta($thumb_id, '_wp_attachment_image_alt', true);
    $logo_id = get_field('logo');
    $logo = $logo_id['sizes']['grid-thumb'];
    $logo_alt = get_post_meta($logo_id, '_wp_attachment_image_alt', true);
    ?>

    <div class="testimonial"><div class="wrap">
        <div class="header">
            <div class="headshot">
               <img src="<?php echo $thumbnail[0]; ?>" alt="<?php echo $thumb_alt; ?>">
            </div>
            <p class="subheading"><?php the_title(); ?></p>
        </div>
        <div class="text">
            <?php the_content(); ?>
        </div>
        <?php if ( is_page_template('startups-page.php') ) : ?>
            <img class="logo" src="<?php echo $logo; ?>"> <!-- needs an alt attribute -->
        <?php endif; ?>
    </div></div>

<?php endwhile; wp_reset_query(); ?>

featured-cs

<?php
/* Display selected case studies (a custom post type). Items are chosen in a custom field for this page in the backend. */
?>

<section class="featured case-studies">

    <h1><?php _e('Case Studies','ssae'); ?></h1>

    <div class="gallery"><div class="container">

        <?php if( have_rows('case_studies')):
            while ( have_rows('case_studies')) : the_row();
                $post_object = get_sub_field('post');
                if( $post_object ) :
                    $post = $post_object;
                    setup_postdata($post);
                    get_template_part('partials/article');
                    wp_reset_postdata();

                endif;

           endwhile; 
        endif; ?>


    </div></div>

</section>

article

<?php
/* Layout for displaying a thumbnail, title, link, and categories for an article. */
?>

<a class="article" href="<?php the_permalink() ?>">
    <div class="image">
        <?php if ( has_post_thumbnail() ) : 
            echo get_the_post_thumbnail( $post->ID, 'grid-thumb');
            else : ?>
            <img src="https://placehold.it/600x440" alt="placeholder" />
        <?php endif; ?>
    </div>
    <h2><?php the_title(); ?></h2>
    <span class="category">
        <?php
        /* show a comma-separated list of associated categories as just text, no links */
        $cats = '';
        foreach((get_the_category()) as $category) {
            if ($category->cat_name != "Uncategorised") { // don't print this "category"
                $cats .= $category->cat_name . ', ';
            }
        }
        echo rtrim($cats, ', ');
        ?>
    </span>
</a>
\$\endgroup\$
  • 1
    \$\begingroup\$ So what exactly is your question? It is unclear what you are asking the review to provide. \$\endgroup\$ – pacmaninbw Sep 16 '16 at 21:05
  • \$\begingroup\$ Ah yes, I had a question but it was edited out by another user! (@Jamal) This is my first post on here... I am applying for (wordpress dev) jobs and putting together code samples to send along. I'm just wondering if the code looks good and legible? \$\endgroup\$ – kanga Sep 16 '16 at 21:33
  • 1
    \$\begingroup\$ But your latest edit removed the main question at the bottom before the first comment. \$\endgroup\$ – Jamal Sep 16 '16 at 22:43
  • \$\begingroup\$ Hmm no the main question was if the code looks good. There was a second question that asked if I should also add more of the code in, but in the edit I added that extra code so I removed that second question since it wasn't relevant anymore. I'm not sure why you removed the original question – should I add it back? @Jamal \$\endgroup\$ – kanga Sep 17 '16 at 23:37
  • \$\begingroup\$ Yeah, go ahead. I mainly removed it because it was implied (and your second question was there), but now it's receiving close votes. \$\endgroup\$ – Jamal Sep 18 '16 at 1:09
3
\$\begingroup\$

I wanted to give some thoughts around coding style. I know you are working in Wordpress which, as a code base, does a relatively poor job in reinforcing good coding style. So if, any of my comments are based on your particular desire to maintain consistency with Wordpress coding style, you can feel free to ignore.


Don't drop in and out of PHP unnecessarily.

This following code example:

<?php get_template_part('partials/callout'); ?>

<?php get_template_part('partials/featured-cs'); ?>

<?php get_footer(); ?>

Would better be written as:

<?php
get_template_part('partials/callout');
get_template_part('partials/featured-cs');
get_footer();

There is way too much vertical whitespace. Why the empty lines (which can actually be a significant problem in PHP), before opening PHP tags?


Think about your code indentation. Look at this section:

    <?php 
        while ( have_rows('stats') ) : the_row();
        $label = get_sub_field('label');
        $value = get_sub_field('value');
    ?>

Why is while() indented out so far when this is part of primary code path for the page? Why is the_row() not on its own line? Why are the following variable assignments not indented; they are clearly within the while() loop?

Perhaps this should look like:

<?php 
while ( have_rows('stats') ) :
    the_row();
    $label = get_sub_field('label');
    $value = get_sub_field('value');
?>

This same thought applies to your code in details partial as well.


It is generally bad to start code directly after PHP tag, or close PHP tag after a line of code (a good exception to this being the case of quick variable insertions within HTML.

But a section of code like the following:

    <?php if( have_rows('case_studies')):
        while ( have_rows('case_studies')) : the_row();
            $post_object = get_sub_field('post');
            if( $post_object ) :
                $post = $post_object;
                setup_postdata($post);
                get_template_part('partials/article');
                wp_reset_postdata();

            endif;

       endwhile; 
    endif; ?>

Might look cleaner like this:

<?php
    if( have_rows('case_studies')):
        while ( have_rows('case_studies')) :
            the_row();
            $post_object = get_sub_field('post');
            if( $post_object ) :
                $post = $post_object;
                setup_postdata($post);
                get_template_part('partials/article');
                wp_reset_postdata();
            endif;
       endwhile; 
    endif;
?>

Here you have clear understanding of nesting level if conditionals and loops that was more obfuscated in previous code.


Consider leaving off closing PHP tags at end of file. Especially with your not very consistent approach to eliminating spurious whitespace in the output, having PHP closing tags, potentially with spurious whitespace after them if probably not a good idea.


Another good example of many of the above points:

<?php 
    $loop = new WP_Query( array( 
        'post_type' => 'testimonials',
        'posts_per_page' => 3
    ) ); 
    while ( $loop->have_posts() ) : $loop->the_post(); 
?>
    <?php
    $thumb_id = get_post_thumbnail_id($post->id);
    $thumbnail = wp_get_attachment_image_src( $thumb_id, 'thumbnail' );
    $thumb_alt = get_post_meta($thumb_id, '_wp_attachment_image_alt', true);
    $logo_id = get_field('logo');
    $logo = $logo_id['sizes']['grid-thumb'];
    $logo_alt = get_post_meta($logo_id, '_wp_attachment_image_alt', true);
    ?>

Here you unnecessarily drop in and out of PHP and indentation is not clear. I would suggest this should be:

<?php 
$loop = new WP_Query( array( 
    'post_type' => 'testimonials',
    'posts_per_page' => 3
 ) ); 
while ( $loop->have_posts() ) :
    $loop->the_post(); 
    $thumb_id = get_post_thumbnail_id($post->id);
    $thumbnail = wp_get_attachment_image_src( $thumb_id, 'thumbnail' );
    $thumb_alt = get_post_meta($thumb_id, '_wp_attachment_image_alt', true);
    $logo_id = get_field('logo');
    $logo = $logo_id['sizes']['grid-thumb'];
    $logo_alt = get_post_meta($logo_id, '_wp_attachment_image_alt', true);
?>
\$\endgroup\$
  • \$\begingroup\$ Thanks – very thorough! I see what you mean, especially about going in and out of php. I think mixing php with html always looks sort of awkward no matter what you do. I'm not sure if I'm sold on using more line breaks, but I'm definitely going to consider it. \$\endgroup\$ – kanga Sep 21 '16 at 22:25
  • \$\begingroup\$ what do you mean with There is way too much vertical whitespace. Why the empty lines (which can actually be a significant problem in PHP), before opening PHP tags? \$\endgroup\$ – pppp Sep 28 '16 at 18:04
  • \$\begingroup\$ @pppp There is an extra line between darn near every HTML section and between blocks of PHP code that simply don't need to be there. Vertical whitespace is important in code clarity, as it provides a useful way to group sections of code. But, when you have vertical whitespace after almost every element or code block on the page, it is hard to see logically grouped sections of code. \$\endgroup\$ – Mike Brant Sep 28 '16 at 21:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.