HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpModerate

Listing WordPress posts with thumbnail images

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withwordpresslistingimagespoststhumbnail

Problem

I am using the following code poor form. I am interested in how I can optimise this and use the correct methods. I understand the DRY concept and will look to employ that throughout my code. However, this works and its pretty quick too, even with large result sets. I would like to learn and improve this code.

```
ID;
$blogUrl = get_site_url();
global $wpdb;
$DBquery = "
SELECT *
FROM saved_collection
WHERE userid = '".$user_id."'
";

$result = $wpdb->get_results($DBquery);

$ids = array();
foreach($result as $row)
{

$collectionID = $row->id;
$collection_name = $row->collection_name;
$collectionURL = 'Programme Link';

echo '';
echo '' . $collection_name . '';
echo $collectionURL;
echo '';
$cleandata = unserialize($row->collection_data);

foreach($cleandata as $data)
{
$ids[] = $data['ExerciseID'];
// START OF QUERY TO GET THE FIRST IMAGE FROM EACH EXERCISE
global $wpdb;
$query = "
SELECT *
FROM imagemap
WHERE exercise = '".$data['ExerciseID']."'
LIMIT 1
";

$result = $wpdb->get_results($query);

foreach($result as $row)

{
echo '';
// START OF QUERY TO GET NAME OF EACH IMAGE
global $wpdb;
$exerciseTitleQuery = "
SELECT post_title
FROM wp_posts
WHERE id = '".$data['ExerciseID']."'
LIMIT 1
";
$exerciseTitleQueryResult = $wpdb->get_results($exerciseTitleQuery);
foreach ($exerciseTitleQueryResult as $returned_post_title)
{echo '';
echo '' .$returned_post_title->post_title. '';
echo '';
}
// END OF QUERY TO GET NAME OF EACH IMAGE

echo "id. ".jpg />";
//

Solution

One of the nested loops can be eliminated, by joining these two queries:

$query = "
SELECT * 
FROM imagemap
WHERE exercise = '".$data['ExerciseID']."'
LIMIT 1
";

$result = $wpdb->get_results($query); 

foreach($result as $row) {
    echo '';
    global $wpdb;
    $exerciseTitleQuery = "
    SELECT post_title 
    FROM wp_posts
    WHERE id = '".$data['ExerciseID']."'
    LIMIT 1
    ";


This code is inefficient, because for every row in the first query, you run another query. For example if there are 10 rows in the first query, you'll be running 11 queries. Queries can be expensive, it's better to minimize them as much as possible, for example by selecting multiple rows in bulk instead of doing it one by one. Since in both the outer and the inner queries the parameter is the same, $data['ExerciseID'], you should be able to use a JOIN instead in a single query.

Another problem you have in the first query is that SELECT is not recommended. You should specify exactly the columns you need, and their order. When you use SELECT , you might be selecting too much, downloading more data than you really need, and if the order columns change in the database or new columns are added, you could have nasty bugs.

Code Snippets

$query = "
SELECT * 
FROM imagemap
WHERE exercise = '".$data['ExerciseID']."'
LIMIT 1
";

$result = $wpdb->get_results($query); 

foreach($result as $row) {
    echo '<div class="imageGridImages imageGridImagesLarge">';
    global $wpdb;
    $exerciseTitleQuery = "
    SELECT post_title 
    FROM wp_posts
    WHERE id = '".$data['ExerciseID']."'
    LIMIT 1
    ";

Context

StackExchange Code Review Q#63505, answer score: 12

Revisions (0)

No revisions yet.