I hope you don't mind, but I took the liberty of doing a quick code review of the plugin. I've added @todo
comments in the source, and here's the diff:
diff --git a/dynamic-featured-image.php b/dynamic-featured-image.php
index ff7e141..b545ca7 100644
--- a/dynamic-featured-image.php
+++ b/dynamic-featured-image.php
@@ -74,7 +74,7 @@ class Dynamic_Featured_Image {
add_action( 'plugins_loaded', array( $this, 'load_plugin_textdomain' ) );
//handle ajax request
- add_action( 'wp_ajax_nopriv_dfiMetaBox_callback', array( $this, 'ajax_callback' ) );
+ add_action( 'wp_ajax_nopriv_dfiMetaBox_callback', array( $this, 'ajax_callback' ) ); // @todo nopriv action should be removed
add_action( 'wp_ajax_dfiMetaBox_callback', array( $this, 'ajax_callback' ) );
$this->upload_dir = wp_upload_dir();
@@ -132,7 +132,7 @@ class Dynamic_Featured_Image {
public function initialize_featured_box() {
global $post;
- $data = get_post_custom( $post->ID );
+ $data = get_post_custom( $post->ID ); // @todo Why not use get_post_meta( $post->ID, 'dfiFeatured', true )?
$totalFeatured = 0;
$featuredData = array();
@@ -284,10 +284,11 @@ class Dynamic_Featured_Image {
* @return Void
*/
public function ajax_callback() {
+ // @todo lacking cap check?
$featuredId = isset($_POST['id']) ? (int) strip_tags( trim( $_POST['id'] ) ) : null;
- if ( is_null( $featuredId ) ) {
+ if ( is_null( $featuredId ) ) { // @todo The above can simple cast to an int, and instead of is_null() here use empty()
return;
}
@@ -350,7 +351,7 @@ class Dynamic_Featured_Image {
$keys = array_keys( $_POST );
foreach ( $keys as $key ) {
- if ( preg_match( '/dfi_fimageplug-.$/', $key ) ) {
+ if ( preg_match( '/dfi_fimageplug-.$/', $key ) ) { // @todo /dfi_fimageplug-.$/ should probably be /dfi_fimageplug-\d+$/
//Verify nonce
if ( !wp_verify_nonce( $_POST[$key], plugin_basename(__FILE__) ) ) {
return;
@@ -384,7 +385,7 @@ class Dynamic_Featured_Image {
/** Helper functions */
private function execute_query( $query ) {
- $resultSet = $this->db->get_col( $query );
+ $resultSet = $this->db->get_col( $query ); // @todo why not use $wpdb->get_col() directly? And actually, $wpdb->get_var() is what you want here.
return empty( $resultSet ) ? null : $resultSet[0];
}
@@ -402,7 +403,7 @@ class Dynamic_Featured_Image {
*/
public function get_image_id( $image_url ) {
- return self::execute_query($this->db->prepare( "SELECT ID FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ));
+ return self::execute_query($this->db->prepare( "SELECT ID FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url )); // @todo should use $wpdb->posts and value should be cached
} // END get_image_id()
@@ -462,7 +463,7 @@ class Dynamic_Featured_Image {
//check if the image is edited image
//and try to get the attachment id
$image_url = str_replace($this->upload_url . "/", '', $image_url);
- $row = self::execute_query( $this->db->prepare( "SELECT post_id, meta_key FROM " . $this->prefix . "postmeta WHERE meta_value = %s", $image_url ) );
+ $row = self::execute_query( $this->db->prepare( "SELECT post_id, meta_key FROM " . $this->prefix . "postmeta WHERE meta_value = %s", $image_url ) ); // @todo should use $wpdb->postmeta instead and value should be cached
if( !is_null($row) ) {
$attachment_id = $row['post_id'];
}
@@ -483,7 +484,7 @@ class Dynamic_Featured_Image {
*/
public function get_image_title( $image_url ) {
- return self::execute_query( $this->db->prepare( "SELECT post_title FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ) );
+ return self::execute_query( $this->db->prepare( "SELECT post_title FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ) ); // @todo should use $wpdb->posts and value should be cached
} // END get_image_title()
@@ -499,7 +500,7 @@ class Dynamic_Featured_Image {
*/
public function get_image_title_by_id( $attachment_id ) {
- return self::execute_query( $this->db->prepare( "SELECT post_title FROM " . $this->prefix . "posts" . " WHERE ID = %d", $attachment_id ) );
+ return self::execute_query( $this->db->prepare( "SELECT post_title FROM " . $this->prefix . "posts" . " WHERE ID = %d", $attachment_id ) ); // @todo should use get_post( $attachnent_id )->post_title since it is cached
} // END get_image_title_by_id()
@@ -531,7 +532,7 @@ class Dynamic_Featured_Image {
*/
public function get_image_caption_by_id( $attachment_id ) {
- return self::execute_query( $this->db->prepare("SELECT post_excerpt FROM " . $this->prefix . "posts" . " WHERE ID = %d", $attachment_id) );
+ return self::execute_query( $this->db->prepare("SELECT post_excerpt FROM " . $this->prefix . "posts" . " WHERE ID = %d", $attachment_id) ); // @todo should use get_post( $attachnent_id )->post_excerpt since it is cached
} // END get_image_caption_by_id()
@@ -549,7 +550,7 @@ class Dynamic_Featured_Image {
*/
public function get_image_alt( $image_url ) {
- $attachment = $this->db->get_col( $this->db->prepare( "SELECT ID FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ) );
+ $attachment = $this->db->get_col( $this->db->prepare( "SELECT ID FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ) ); // @todo should use $wpdb->posts and value should be cached
$alt = null;
if ( !empty( $attachment ) ) {
@@ -592,7 +593,7 @@ class Dynamic_Featured_Image {
*/
public function get_image_description( $image_url ) {
- return self::execute_query( $this->db->prepare( "SELECT post_content FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ) );
+ return self::execute_query( $this->db->prepare( "SELECT post_content FROM " . $this->prefix . "posts" . " WHERE guid = %s", $image_url ) ); // @todo should use $wpdb->posts and value should be cached
} // END get_image_description()
@@ -607,8 +608,8 @@ class Dynamic_Featured_Image {
* @return String
*/
public function get_image_description_by_id( $attachment_id ) {
-
- return self::execute_query( $this->db->prepare( "SELECT post_content FROM " . $this->prefix . "posts" . " WHERE ID = %d", $attachment_id ) );
+
+ return self::execute_query( $this->db->prepare( "SELECT post_content FROM " . $this->prefix . "posts" . " WHERE ID = %d", $attachment_id ) ); // @todo should use get_post( $attachnent_id )->post_content since it is cached
} // END get_image_description_by_id()
@@ -627,12 +628,12 @@ class Dynamic_Featured_Image {
public function get_post_attachment_ids( $post_id ) {
$dfiImages = get_post_custom( $post_id );
- $dfiImages = ( isset( $dfiImages['dfiFeatured'][0] ) ) ? @array_filter( unserialize( $dfiImages['dfiFeatured'][0] ) ) : array();
+ $dfiImages = ( isset( $dfiImages['dfiFeatured'][0] ) ) ? @array_filter( unserialize( $dfiImages['dfiFeatured'][0] ) ) : array(); // @todo shouldn't be suppressing errors with @
$retVal = array();
if ( !empty( $dfiImages ) && is_array( $dfiImages ) ) {
foreach ( $dfiImages as $dfiImage ) {
- @list( $dfiImageTrimmed, $dfiImageFull ) = explode( ',', $dfiImage );
+ @list( $dfiImageTrimmed, $dfiImageFull ) = explode( ',', $dfiImage ); // @todo shouldn't be suppressing errors with @
$retVal[] = $this->get_image_id( $this->upload_url . $dfiImageFull );
}
@@ -656,7 +657,7 @@ class Dynamic_Featured_Image {
* @return Array if found, null otherwise
*/
public function get_nth_featured_image( $position, $post_id = null ) {
-
+ // @todo function is not indented properly
if ( is_null( $post_id ) ) {
global $post;
$post_id = $post->ID;
diff --git a/js/script-dfi.js b/js/script-dfi.js
index ec9973c..254177f 100644
--- a/js/script-dfi.js
+++ b/js/script-dfi.js
@@ -5,6 +5,7 @@
*
* Copyright (c) 2013, Ankit Pokhrel <[email protected], http://ankitpokhrel.com.np>
*/
+ // @todo file should be using tabs for indentation; there is inconsistent indentation with spaces
jQuery(document).ready(function($){
var current = null;