JOIN and sprintf rather than two queries and echo

It works fine, however it's a little messy. I understand it would be better for performance to use JOIN and have been advised before to use sprintf. However, it's pretty confusing and I cannot understand the PHP tutorials on it.

get_results($getImageIdQuery);

foreach($imageID as $row2) {
 }

$mappedImage = $row2->image_map;

$query = "
SELECT id 
FROM imagemap
WHERE exercise = '".$mappedImage."'";

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

foreach($result as $row) {
 }
echo '';
echo '' .the_title(). '';
echo '';
echo "id. ".jpg />";

?>

Escaping Variable Part of Query

Check out this and this post about correctly using the wordpress database class to avoid SQL injection. Even if the variable data is not directly user supplied, I would always use prepared statements or at least escape data as defense in depth, and to prevent second order SQL injection.

Your code should look something like this:

$getImageIdQuery = "
SELECT image_map
FROM wp_posts
WHERE ID = %d";

$sql = $wpdb->prepare( $getImageIdQuery, $exerciseID );
$imageID = $wpdb->get_results($sql);

Same goes for the other query (the added benefit is that this way, you don't need sprintf because the SQL code and the PHP code are already separated).

Escaping Variable Part of Output

I would also sanitize all variable data when echoing it to the user. Right now, anybody who can create posts can include arbitrary javascript via the_title. This might actually be ok, but still, to be save I would write it like this:

echo '';
echo '' . esc_html(the_title()) . '';
echo '';
echo "id. ".jpg") . "/>";

Foreach

You have two foreach statements which do not do anything. but their side-effect is that your are always only working on the last entry retrieved by the query.

Instead of returning all matching entries, you should only retrieve this one entry and remove your foreach loops (you can use get_row (if needed in combination with limit and order by)).

Misc

• I would use more spaces to make the code more readable (for example around .). See also the wordpress style guide.

• use single and double quotes consistently (for example "'; both should use single quotes)

sprintf

I don't really like using sprintf, but if you want to, it would look like this:

echo '';
echo sprintf('%s', esc_html(the_title()));
echo '';
echo sprintf("", esc_url($blogUrl . "/image/medium/".$row->id. ".jpg"));

%s is a placeholder for any string (you would use %d for numbers), and it will be replaced by the rest of the arguments for sprintf.

The advantage is that you do not have PHP variables/function calls inside your HTML/SQL/etc.

join

You can read about join here and a bit more in-depth here.

Your query should look something like this:

SELECT imagemap.id FROM imagemap JOIN wp_posts ON imagemap.exercise = wp_posts.image_map WHERE wp_posts.ID = $exerciseID

What it basically does is combine the two tables (based on the ON clause).

For example, if the tables looked like this:

imagemap:
id exercise
1  1
2  2
3  2
4  3

wp_posts:
pid image_map
6   1
7   1
8   2

the combined table would look like this:

id exercise pid image_map
1  1        6   1
1  1        7   1 
2  2        8   2
3  2        8   2

and then the where clause is applied (I called wp_posts.ID pid for the example).

StackExchange Code Review Q#70668, answer score: 3