WordPress/PHP - Refining an if/else statement

Question

I am looking at adding an if/else statement, showing different HTML as required. My question is how I could possibly refine this code? Is there a more refined way of presenting this code?

<?php if ('Images' == get_post_meta($post->ID, 'project_type', true)) { ?>
                  <h1>Images</h1>
<div class="images">...</div>
                <? } elseif ('Slideshow' == get_post_meta($post->ID, 'project_type', true)) { ?>
                  <h1>Slideshow</h1>
<div class="slideshow">...</div>
                <? } elseif ('Video' == get_post_meta($post->ID, 'project_type', true)) { ?>
                    <h1>Video</h1>
<div class="video">...</div>
                <? } elseif ('Audio' == get_post_meta($post->ID, 'project_type', true)) { ?>
                    <h1>Audio</h1>
<div class="audio">...</div>
                <?php } ?>

Fleshgrinder · Accepted Answer · 2014-02-11 00:58:07Z

1

<?php

// Only call the function once (performance)
$post_id = get_post_meta($post->ID, "project_type", true);

// Use a whitelist to validate
$whitelist = array("Images", "Slideshow", "Video", "Audio");

// Check if given post ID is valid
if (in_array($post_id, $whitelist) === true) { ?>
    <h1><?= $post_id ?></h1>
    <div class="<?= strtolower($post_id) ?>">...</div>
<?php } ?>

Yes, this approach doesn't take into account what might be happening within your <div> element. As this isn't part of your question. If there is a lot going on I'd propose an object oriented approach. Another bad approach would be a switch statement.

<?php

switch (get_post_meta($post->ID, "project_type", true)) {
    case "Images": ?>
        <h1>Images</h1>
        <div class="images">...</div>
        <?php break;

    case "Slideshow": ?>
        <!-- Same story again ... -->
        <?php break;
} // End switch

With OOP we can create something like the following:

<?php

namespace Stackoverflow;

abstract class MyBaseTemplate {

    protected $title;

    protected $class;

    protected $content;

    public function __toString() {
        return "<h1>{$this->title}</h1><div class='{$this->class}'>{$this->content}</div>";
    }

}

class Images extends MyBaseTemplate {

    public function __construct() {
        $this->title   = "Images";
        $this->class   = "images";
        $this->content = "...";
    }

}

class Slideshow extends MyBaseTemplate {

    // Init

}

// In the other file instead of if/else and switch

$post_id = get_post_meta($post->ID, "project_type", true);
$class   = "\\Stackoverflow\\{$post_id}";

if (class_exists($class) === true) {
    echo new $class();
}

edited Feb 11, 2014 at 0:58

answered Feb 10, 2014 at 22:15

Fleshgrinder

16.4k5 gold badges54 silver badges56 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

Barbara Laird Over a year ago

This assumes that what is within the ... is identical across the different types. Which seems unlikely to me.

Fleshgrinder Over a year ago

Well, I don't know what's happening in there. I'd say, go for polymorphism.

Fleshgrinder Over a year ago

Extended my answer to show two different approaches (including OO).

Wayne Malkin · Accepted Answer · 2014-02-10 22:23:35Z

0

Sometimes it can be cleaner to put all that markup in echo statements, but only if it is simple markup and the result is actually more readable than mixing in the php statements as above. Example:

<?php
if (...) {
    echo "<h1>Images</h1>"
    echo "<div class=\"images\">...</div>"
} else if(...) {
    echo "<h1>...</h1>"
}
?>

answered Feb 10, 2014 at 22:23

Wayne Malkin

531 silver badge4 bronze badges

Comments

butlerblog · Accepted Answer · 2014-02-10 22:37:17Z

0

Fleshgrinder is right, you could make this better by only using get_post_meta one time.

If the contents of the html is different for each of these, I would probably use a switch rather than else/elseif/etc...

$project_type = get_post_meta($post->ID, 'project_type', true);
switch ( $project_type ) {

case 'Images':
    echo '<h1>Images</h1>';
    echo '<div class="images">...</div>';
    break;

case 'Slideshow':
    echo '<h1>Slideshow</h1>';
    echo '<div class="slideshow">...</div>';
    break;

case 'Video':
    echo '<h1>Video</h1>';
    echo '<div class="video">...</div>';
    break;

case 'Audio':
    echo '<h1>Audio</h1>';
    echo '<div class="audio">...</div>';
    break;
}

answered Feb 10, 2014 at 22:37

butlerblog

4368 silver badges16 bronze badges

1 Comment

FlavaFlav2013 Over a year ago

Thanks for so many great responses. I should add, that it is a mix of HTML and PHP that is included in the if statements. How does this alter things?

Collectives™ on Stack Overflow

WordPress/PHP - Refining an if/else statement

3 Answers 3

3 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related