in Best Practices, PHP 5

Peer Review: Taking Code And Making It Better

On Wednesday, August 12th, we had a meeting of the DC PHP Developer’s Group. Keith Casey of Blue Parabola led a code review of a member-submitted sample. The review was informative, educational, and helpful. With the permission of that member, I’ve decided to write a series on the tools for reviewing code and re-factoring it.

The code sample (included below) isn’t perfect. It needs work, and we’ll be working on it over the next few articles. Along the way, we’ll talk about strategies for identifying weaknesses, candidates for refactoring, and methods for writing better code before you get to the review stage. We’ll also refactor this into something more usable, and end up with a finished product that’s better than when we started.

The knowledge gained is based off the DC PHP Developer’s Group, as well as many notes that I put together. The refactoring process requires a lot of thought, and so this series will contain six more entries, starting with the first one on Monday. But before we begin, here’s the code sample we’ll be working with:

Warning: This code should not be used in production and is for demonstration purposes only!
<?php // class_Twitter.php - NPC TWITTER AUTO-FEED 
error_reporting(E_ALL); 
 
// SEND AN AUTOMATED TWEET 
// USAGE EXAMPLE 
//   require_once('class_Twitter.php'); 
//   $status = 'Hello World'; 
//   $t      = new Twitter; 
//   $t->tweet($status); 
//   unset($t); 
 
 
// DO NOT RUN THIS SCRIPT STANDALONE (HAS PASSWORD) 
if (count(get_included_files()) < 2) { 
    header("HTTP/1.1 301 Moved Permanently"); header("Location: /"); exit; 
} 
 
class Twitter 
{ 
    private $ch; 
    private $you; 
    private $user; 
    private $pass; 
    private $test; 
    private $host; 
    private $done; // ALREADY TWEETED? 
    public  $res; 
 
    public function __construct() 
    { 
        $this->user = "??? TWITTER USERNAME"; 
        $this->pass = "??? TWITTER PASSWORD"; 
        $this->you  = "??? YOUR EMAIL ADDRESS"; 
 
        $this->host = "http://twitter.com/"; 
        $this->done = FALSE; // DEFAULT - NOT ALREADY TWEETED 
        $this->test = FALSE; // DEFAULT - THIS IS LIVE, NOT A TEST 
 
        $this->ch   = curl_init(); 
        curl_setopt($this->ch, CURLOPT_VERBOSE, 1); 
        curl_setopt($this->ch, CURLOPT_RETURNTRANSFER, 1); 
        curl_setopt($this->ch, CURLOPT_USERPWD, "$this->user:$this->pass"); 
        curl_setopt($this->ch, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1); 
        curl_setopt($this->ch, CURLOPT_POST, 1); 
    } 
 
    public function __destruct() 
    { 
        curl_close($this->ch); 
    } 
 
// SET AN INDICATOR THAT THIS IS NOT A LIVE TWEET 
    public function test() 
    { 
        $this->test = TRUE; 
    } 
 
// DETERMINE IF THE MESSAGE HAS ALREADY BEEN SEND 
    private function already_tweeted($message) 
    { 
        $text     = mysql_real_escape_string(trim($message)); 
        $date     = date('Y-m-d'); 
        $code     = md5($text . $date); 
        $sql      = "SELECT id FROM twitterLog WHERE thash = \"$code\" ORDER BY id DESC LIMIT 1"; 
        if (!$res = mysql_query($sql)) die(mysql_error()); 
        $num      = mysql_num_rows($res); 
        if ($num) return TRUE;
        $sql      = "INSERT INTO twitterLog (tdate, thash, tweet) VALUES ( \"$date\", \"$code\", \"$text \" )"; 
        if (!$res = mysql_query($sql)) die(mysql_error()); 
        return FALSE; 
    } 
 
// POST A MESSAGE TO TWITTER 
    public function tweet($message) 
    { 
        if(strlen($message) < 1) return FALSE; 
        if ($this->already_tweeted($message)) $this->done = TRUE; 
 
// IF ONLY A TEST, JUST EMAIL THE INFORMATION - DO NOT TWEET 
        if ($this->test) 
        { 
            $msg = ''; 
            if ($this->done) $msg .= "ALREADY DONE "; 
            $msg .= "TWEET: $message"; 
            mail($this->you, 'What We Would Have Tweeted', $msg); 
            return TRUE; 
        } 
 
// DO NOT REPEAT YOURSELF 
        if ($this->done) return TRUE; 
 
// STATUS UPDATE ON TWITTER 
        $this->host .= "statuses/update.xml?status=".urlencode( stripslashes( urldecode($message ))); 
        curl_setopt($this->ch, CURLOPT_URL, $this->host); 
        $xxx       = curl_exec($this->ch); 
        $this->res = curl_getinfo($this->ch); 
        if ($this->res['http_code'] == 0)   return TRUE; 
        if ($this->res['http_code'] == 200) return TRUE; 
        return FALSE; 
    } 
} 
?>

Frustrated with your company’s development practices?

You don't have to be!

No matter what the issues are, they can be fixed. You can begin to shed light on these issues with my handy checklist.

Plus, I'll help you with strategies to approach the issues at the organization level and "punch above your weight."

Powered by ConvertKit