Grossly inefficient wordpress loops!

Question

I'm working on a site at the moment for an orchestra. The various members need to be listed, according to their instrument. The members have a custom post type of biography and I'm capturing the instrument value via a custom field.

The only way I can figure out how to display the relevant people in their relevant sections is to loop again and again through the custom post type, displaying the people that play a particular instrument by comparing the meta value.

Code looks like this:

<?php $args = array( 'post_type' => 'biographies', 'posts_per_page' => -1 ); ?>

    <ul class="no-bull hijax">
        <?php $biog = new WP_Query($args);
            if( $biog->have_posts() ) : while( $biog->have_posts() ) : $biog->the_post();
                $player = get_post_meta($post->ID, 'player', true);
                if ($player == 'yes') :
                    $instrument = get_post_meta($post->ID, 'instrument', true);
                    if ($instrument == 'violin') :
                    ?>
                        <li><a id="artist_id_<?php the_ID(); ?>" class="nb" href="<?php the_permalink(); ?>"><?php the_title(); ?></a> : <?php echo($instrument); ?></li>
                    <?php
                    endif;
                endif;
            endwhile; endif;
        wp_reset_query();
        $biog = new WP_Query($args);
            if( $biog->have_posts() ) : while( $biog->have_posts() ) : $biog->the_post();
                $player = get_post_meta($post->ID, 'player', true);
                if ($player == 'yes') :
                    $instrument = get_post_meta($post->ID, 'instrument', true);
                    if ($instrument == 'viola') :
                    ?>
                        <li><a id="artist_id_<?php the_ID(); ?>" class="nb" href="<?php the_permalink(); ?>"><?php the_title(); ?></a> : <?php echo($instrument); ?></li>
                    <?php
                    endif;
                endif;
            endwhile; endif;
        wp_reset_query(); 
        $biog = new WP_Query($args);
            if( $biog->have_posts() ) : while( $biog->have_posts() ) : $biog->the_post();
                $player = get_post_meta($post->ID, 'player', true);
                if ($player == 'yes') :
                    $instrument = get_post_meta($post->ID, 'instrument', true);
                    if ($instrument == 'cello') :
                    ?>
                        <li><a id="artist_id_<?php the_ID(); ?>" class="nb" href="<?php the_permalink(); ?>"><?php the_title(); ?></a> : <?php echo($instrument); ?></li>
                    <?php
                    endif;
                endif;
            endwhile; endif;
        wp_reset_query();

etc. etc. ad nauseum. (there are currently 12 loops on the page!!)

This is clearly totally inefficient, but quite simply, I don't know how to write better code that this and need some help!

Answer 1

You could do it with one loop, you just need a valid sort order right? Players with one instrument, followed by the next and so on..

UPDATE: Following on the asker's comment, you can still use one query and use rewind_posts() to iterate the loop as many times as you need, ie. do something like this to get a custom sort..

<?php
// Add the instruments into the array below, in the order you want them in.
$instruments = array( 'violin', 'viola', 'cello' );

$args = array( 
    'post_type' => 'biographies', 
    'posts_per_page' => -1,
    'nopaging' => true,
    'surpress_filters' => true,
    'meta_query' => array(
        array(
            'key' => 'player',
            'value' => 'yes',
            'compare' => '=',
            'type' => 'CHAR'
        ),
        array(
            'key' => 'instruments',
            'value' => $instruments,
            'compare' => 'IN',
            'type' => 'CHAR'
        )
    ),
); 

$bios = new WP_Query( $args);
?>

<?php if( $bios->have_posts() ) : ?>

    <ul class="no-bull hijax">

    <?php
    foreach( $instruments as $instrument ) :

        while( $bios->have_posts() ) : $bios->the_post();

            $player_instrument = get_post_meta( get_the_ID(), 'instrument', true );

            if( $instrument != $player_instrument )
                continue;
        ?>

        <li><a id="artist_id_<?php the_ID(); ?>" class="nb" href="<?php the_permalink(); ?>"><?php the_title(); ?></a> : <?php echo $player_instrument; ?></li>

        <?php 

        endwhile; 
        rewind_posts();

    endforeach;
    wp_reset_query();
    ?>

    </ul>

<?php endif; ?>

See if that has the desired effect.. :)

Answer 2

I would also recommend using custom taxonomies. An intersection between 'Instrument' and 'Player' taxonomies would make short work of this. But if that's not practical, perhaps this might work:

$args = array(
  'post_type' => 'biographies',
  'posts_per_page' => -1,
  'meta_key' => 'instrument',
  'orderby' => 'meta_value'
);

query_posts($args);

while(have_posts()) : the_post();
  $inst = get_post_meta($post_id, 'instrument', true);
  $player = get_post_meta($post_id, 'player', true);
  if ('yes' == $player) {
  ?>
    <li><a id="artist_id_<?php the_ID(); ?>" class="nb" href="<?php the_permalink(); ?>"><?php the_title(); ?></a> : <?php echo $inst; ?></li>
  <?php
  }

endwhile;

Answer 3

Ehm... you don't need to write a loop again and again. Only work on the part that get's changed. The loop will through all datasets anyway. Btw: I wouldn't make custom fields for this. A "instruments" taxonomy with "violin", "viola", etc. as term would be much easier and give you much more possibilities (namely template tags), eg. making subtaxs like "wind instruments", "percussion instruments", etc. You should also move the if statement inside of the <li> element, as it doesn't change either. The only thing is the if $instrument == '' and the echo $instrument; (which can be written without the surrounding ()).

   if ($instrument == 'violin') : // only $instrument changes, right?
   ?>
     <li>
        <a id="artist_id_<?php the_ID(); ?>" class="nb" href="<?php the_permalink(); ?>">
           <?php the_title(); ?>
        </a> : <?php echo($instrument); ?>
     </li>
   <?php
   elseif ($instrument == 'violin') :
      // do stuff...
   endif;

---

Edit: You could utilize a custom taxonomy named "instruments" and then filter you query in the desired order:

// @link: http://codex.wordpress.org/Function_Reference/taxonomy_exists  
if ( taxonomy_exists('instruments') ) :
  // @link: http://codex.wordpress.org/Function_Reference/has_term
  if ( has_term( 'instruments', 'viola', get_post_ID() ) ): 
    # DO STUFF HERE, eg. echo term
  endif;
endif;

Just make one loop and inside the while-loop just question the term and echo/display stuff. You could, instead of using the if ( has_term(, also use a switch for more readability.

Using this solution avoids further DB calls with get_post_meta() - afaik this calls the DB and not the post object - for every instrument.