Your question is not really about WordPress, it is more about PHP and refactoring. But we see so much bad code here, and the pattern I will explain below (MVC) could help many other developers, so I decided to write a little answer. Keep in mind, there is a dedicated site for such questions in our network: Code Review. Unfortunately, very few WordPress developers are active there.
How to refactor code
- Remove useless code. Beautify the rest.
- Find all repeating expressions and create routines (functions or classes) to
abstract and encapsulate those. - Separate data handling, the model (store, fetch, conversion, interpretation),
from output, the view (HTML, CSV, whatever).
1. Remove useless code. Beautify the rest.
The output
You have this repeating snippet:
if( $query->have_posts()) : while( $query->have_posts() ) : $query->the_post();
the_post_thumbnail('thumbnail');
endwhile;
endif;
You run the rather expensive the_post() each time to get the post thumbnail.
But that’s not needed, you can just call:
echo get_the_post_thumbnail( $post_id, 'post-thumbnail' );
The query
So all you need is the post ID, and this is available without calling the_post().
Even better: you can restrict the query to fetch just the IDs.
A simple example:
$post_ids = array();
$args = array(
'post_type' => 'post',
'posts_per_page' => 10,
'fields' => 'ids'
);
$query = new WP_Query( $args );
if ( ! empty ( $query->posts ) )
$post_ids = $query->posts; // just the post IDs
Now you have the IDs, and you can write:
foreach ( $post_ids as $post_id )
echo get_the_post_thumbnail( $post_id, 'post-thumbnail' );
No overhead, your code is already faster and easier to read.
The syntax
Note how I aligned the =? This helps understanding the code, because the human
mind is specialized in pattern recognition. Support that and we can do awesome
things. Create a mess, and we get stuck very fast.
This is also the reason why I removed endwhile and endif. The alternative
syntax is messy and hard to read. Plus, it makes working in an IDE much harder:
folding and jumping from the start to the end of an expression is easier with
braces.
The default values
Your $args array has some fields you use everywhere. Create a default array,
and write those fields just once:
$args = array(
'post_type' => 'product',
'posts_per_page' => 100,
'fields' => 'ids',
'tax_query' => array(
array(
'taxonomy' => 'product_cat',
'field' => 'slug',
)
)
);
Again, note the alignment. And note also how I changed the posts_per_page value.
Never ask for -1. What happens when there are one million matching posts?
You don’t want to kill your database connection each time this query runs, do you?
And who should read all these posts? Always set a reasonable limit.
Now all you have to change is the field $args[ 'tax_query' ][ 'terms' ]. We will cover that in a moment.
2. Find all repeating expressions and create routines
We cleaned up some repeating code already, now the hard part: the evaluation of
POST parameters. Obviously, you have made some labels as a result of some
parameters. I suggest to rename those to something easier to understand, but for
now we will work with your naming scheme.
Separate these groups from the rest, create an array you can manage later separately:
$groups = array(
'fashion-follower' => array(
'q1' => 'party',
'q2' => 'clothes',
'q3' => 'shopping',
'q4' => FALSE,
'q5' => 'sunbathing',
'q6' => 'mini',
),
'the-homemaker' => array(
'q1' => 'drink',
'q2' => 'candles',
'q3' => 'house',
'q4' => 'diy',
'q5' => FALSE,
'q6' => FALSE,
)
);
To fill the missing terms field in your default array, you run through the
$groups array until you find a match:
function get_query_term( $groups )
{
foreach ( $groups as $term => $values )
{
if ( compare_group_values( $values ) )
return $term;
}
return FALSE;
}
function compare_group_values( $values )
{
foreach ( $values as $key => $value )
{
// Key not sent, but required
if ( empty ( $_POST[ $key ] ) and ! empty ( $value ) )
return FALSE;
// Key sent, but wrong value
if ( ! empty ( $_POST[ $key ] ) and $_POST[ $key ] !== $value )
return FALSE;
}
// all keys matched the required values
return TRUE;
}
I separated even the run through the term list and the comparison of the values,
because these are different operations. Every part of your code should do just
one thing, and you have to keep the indentation level flat for better readability.
Now we have all the parts, let’s stick them together.
3. Organization: Separate the model from the view
When I wrote model and view, I had something in mind: the MVC approach.
It stands for Model View Controller, a well known pattern to organize software
components. The missing part so far was the controller, we will see how we use it later.
You said, you don’t know much about PHP, so I hope you know more about the output. 🙂
Let’s start with that:
class Thumbnail_List
{
protected $source;
public function set_source( Post_Collector_Interface $source )
{
$this->source = $source;
}
public function render()
{
$post_ids = $this->source->get_post_ids();
if ( empty ( $post_ids ) or ! is_array( $post_ids ) )
return print 'Nothing found';
foreach ( $post_ids as $post_id )
echo get_the_post_thumbnail( $post_id, 'post-thumbnail' );
}
}
Nice and simple: we have two methods: one to set the source for our post IDs,
one to render the thumbnails.
You might wonder what this Post_Collector_Interface is. We get to that in a moment.
Now the source for our view, the model.
class Post_Collector implements Post_Collector_Interface
{
protected $groups = array();
public function set_groups( Array $groups )
{
$this->groups = $groups;
}
public function get_post_ids()
{
$term = $this->get_query_term();
if ( ! $term )
return array();
return $this->query( $term );
}
protected function query( $term )
{
$args = array(
'post_type' => 'product',
'posts_per_page' => 100,
'fields' => 'ids',
'tax_query' => array(
array(
'taxonomy' => 'product_cat',
'field' => 'slug',
'terms' => $term
)
)
);
$query = new WP_Query( $args );
if ( empty ( $query->posts ) )
return array();
return $query->posts;
}
protected function get_query_term()
{
foreach ( $this->groups as $term => $values )
{
if ( compare_group_values( $values ) )
return $term;
}
return FALSE;
}
protected function compare_group_values( $values )
{
foreach ( $values as $key => $value )
{
// Key not sent, but required
if ( empty ( $_POST[ $key ] ) and ! empty ( $value ) )
return FALSE;
// Kent sent, but wrong value
if ( ! empty ( $_POST[ $key ] ) and $_POST[ $key ] !== $value )
return FALSE;
}
// all keys matched the required values
return TRUE;
}
}
This is not so trivial anymore, but we had the most parts already. The protected methods (functions) are not accessible from the outside, because we need them for the internal logic only.
The public methods are simple: the first gets our $group array from above,
the second returns an array of post IDs. And again we meet this dubious Post_Collector_Interface.
An interface is a contract. It can be signed (implemented) by classes. Requiring
an interface, like our class Thumbnail_List does, means: the class expects some other class with these public methods.
Let’s build that interface. It is really simple:
interface Post_Collector_Interface
{
public function set_groups( Array $groups );
public function get_post_ids();
}
Yup, that’s all. Easy code, isn’t it?
What we did here: we made our view Thumbnail_List independent from a concrete
class while we still can rely on the methods of the class we got as $source.
If you change your mind later, you can write a new class to fetch the post IDs
or use one with fixed values. As long as you implement the interface, the view
will be satisfied. You can even test the view now with a mock object:
class Mock_Post_Collector implements Post_Collector_Interface
{
public function set_groups( Array $groups ) {}
public function get_post_ids()
{
return array ( 1 );
}
}
This is very useful when you want to test the view. You don’t want to test both
concrete classes together, because the you wouldn’t see where an error comes from.
The mock object is too simple for errors, ideal for unit tests.
Now we have to combine our classes somehow. This is where the controller enters
the stage.
class Thumbnail_Controller
{
protected $groups = array(
'fashion-follower' => array(
'q1' => 'party',
'q2' => 'clothes',
'q3' => 'shopping',
'q4' => FALSE,
'q5' => 'sunbathing',
'q6' => 'mini',
),
'the-homemaker' => array(
'q1' => 'drink',
'q2' => 'candles',
'q3' => 'house',
'q4' => 'diy',
'q5' => FALSE,
'q6' => FALSE,
)
);
public function __construct()
{
// not a post request
if ( 'POST' !== $_SERVER[ 'REQUEST_METHOD' ] )
return;
// set up the model
$model = new Post_Collector;
$model->set_groups( $this->groups );
// prepare the view
$view = new Thumbnail_List;
$view->set_source( $model );
// finally render the tumbnails
$view->render();
}
}
The controller is the one real unique part of an application; the model and the view might be re-used here and there, even in completely different parts. But the controller exists for just this single purpose, this is why we put the $group here.
And now you have to do just one thing:
// Let the dogs out!
new Thumbnail_Controller;
Call this line wherever you need the output.
You can find all the code from this answer in this gist on GitHub.