iwankgb wrote at 8/26/2009 2:50 am:You could use pecl_http package rather the write a wrapper around cURL. It could save you some coding and, being C extension, pecl_http should be more efficient than any PHP wrapper.
| « FIEO: Filtering Input with PHP’s Filter Functions | My Essential WordPress Plugins » |
This entry is part of an ongoing series involving the review of a code sample and it’s refactoring. For the original code sample, see here.
There are a number of fundamental concepts in object-oriented design that we should take notice of. One of these concepts is abstraction. This is what we will focus on today and in the next entry.
This article will focus on the constructor method. There are a couple of problems, namely that the constructor itself does a lot of actual work. Also, we have the cURL setup done in the constructor. This object is a Twitter object, not a cURL object; this means that we should decouple the cURL functionality and abstract it into a separate object of its own. This not only will make our object more true to it’s functionality as a Twitter object, but will allow for greater reuse (since we’ll be able to use the cURL object for more).
There’s some debate about whether or not you ought to write things like wrappers for native PHP functionality. One thing that is missed in the “don’t write a wrapper for cURL!” argument is that a good wrapper for HTTP requests shouldn’t be limited to cURL. In fact, it should make use of fopen(), file_get_contents(), etc. in the case that cURL doesn’t exist. A good object would test for this, and change its behavior based on preset rules. (strategy pattern, anyone?)
There are two things that we need to: first, we need to abstract the cURL tools out of the class. Second, we need to further encapsulate the class. The Constructor should not do the work of cURL setup; this should be handled elsewhere. We will do this by writing a new class (which I will not show) for HTTP requests, and then we will handle the HTTP request in the existing functions. Our constructor will now look like this:
/**
* Constructor.
* @param string $username The username of the Twitter user
* @param string $password The password of the Twitter user
* @param string $email Optional email address for testing
* @return void
*/
public function __construct($username, $password, $email = null)
{
$this->user = $username;
$this->pass = $password;
$this->you = $email;
$this->host = "http://twitter.com/";
$this->done = FALSE; // DEFAULT - NOT ALREADY TWEETED
$this->test = FALSE; // DEFAULT - THIS IS LIVE, NOT A TEST
$this->http = new HTTPRequest();
}
We will assume that the HTTPRequest class, which is not shown, will do error-handling, check to see the appropriate methods are available, and handle configuration items like timeout, host, and other settings.
The next entry in the series will focus on abstracting the database layer, which will be roughly similar to entry, but will contain more information about the reasons for abstraction and the process of abstracting the code.
Our class now looks as follows:
<?php
// class_Twitter.php - NPC TWITTER AUTO-FEED
error_reporting(E_ALL);
// DO NOT RUN THIS SCRIPT STANDALONE (HAS PASSWORD)
if (count(get_included_files()) < 2) {
header("HTTP/1.1 301 Moved Permanently"); header("Location: /"); exit;
}
/**
* Twitter Class
* @author Anonymous
*
*/
class Twitter
{
/**
* @var HTTPRequest HTTPRequest Object.
*/
protected $http;
/**
* @var string Email address for administrator/tester
*/
protected $you;
/**
* @var string The username for the Twitter account.
*/
protected $user;
/**
* @var string The password for the Twitter account.
*/
protected $pass;
/**
* @var bool The flag for test mode
*/
protected $test;
/**
* @var bool Flag for whether or not this tweet has been sent today
*/
protected $done;
/**
* Constructor.
* @param string $username The username of the Twitter user
* @param string $password The password of the Twitter user
* @param string $email Optional email address for testing
* @return void
*/
public function __construct($username, $password, $email = null)
{
$this->user = $username;
$this->pass = $password;
$this->you = $email;
$this->baseHost = "http://twitter.com/";
$this->done = FALSE; // DEFAULT - NOT ALREADY TWEETED
$this->test = FALSE; // DEFAULT - THIS IS LIVE, NOT A TEST
$this->http = new HTTPRequest();
}
/**
* A destructor.
* @return void
*/
public function __destruct()
{
unset($this->http);
}
/**
* Method sets a flag as to whether the object is in test mode
* @return void
*/
public function test()
{
$this->test = TRUE;
}
/**
* Check to see if this tweet has already been posted and add it to the DB
* if it has.
* @param string $message The tweet
* @return bool
*/
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;
}
/**
* A public method to post a tweet. If in test mode, this method will email
* the tweet instead of posting it.
* @param string $message The tweet to be posted.
* @return bool
*/
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->http->setHost($this->baseHost . "statuses/update.xml?status=".urlencode(stripslashes(urldecode($message)));
$this->http->setCredentials($this->user, $this->pass);
$this->http->setOption(HTTPRequest::RETURN_TRANSFER, true);
$this->http->setOption(HTTPRequest::POST, true);
$response = $this->http->execute();
if (($response->statusCode == 0) || ($response->statusCode == 200))
{
return TRUE;
}
return FALSE;
}
}
?>
Brandon Savage is the author of Mastering Object Oriented PHP and Practical Design Patterns in PHP
Posted on 8/26/2009 at 1:00 am
You could use pecl_http package rather the write a wrapper around cURL. It could save you some coding and, being C extension, pecl_http should be more efficient than any PHP wrapper.
Brandon Savage (@brandonsavage) wrote at 8/26/2009 8:25 am:
You’re absolutely right. This PECL package is stable, and object oriented, which makes it a great option.
For those interested, you can check out the package here:
Jeff Carouth (@jcarouth) wrote at 8/26/2009 11:45 am:
For testing purposes you should remove the instantiation of the HttpRequest object from your constructor. Instead you should inject the HttpRequest object. This decouples your Twitter class from the HttpRequest class.
Great series!
If the object were to be used outside, I might allow it to be injected into the Twitter class. However, because I see it as a “use once, throw away” object, it shouldn’t come from the outside.
If I have an HTTP object that I’m using, and I inject it through the constructor, the changes I make to it will be reflected in any link I have to that object anywhere in my application. By creating a new copy of it, I avoid this problem.
Thoughts?
Greg Beaver wrote at 8/26/2009 3:09 pm:
Hi Brandon,
Your refactored example makes the common mistake of using a private method that should be a protected method. By using protected, you retain all of the benefits of information hiding without preventing any future possibility of extension, or worse, a requirement to cut/paste just to extend.
For instance, your already_tweeted() method uses mysql, but another person might choose to use pgsql. In order to implement this, our hypothetical person must completely re-implement the tweet() method just to override already_tweeted() which is clearly antithetical to the basic principles of code re-use.
In terms of the HTTP object, it is better to instantiate it in a separate method, one that can be overridden, again for the same reasons as above as well as ease of unit testing in a non-internet-enabled mock twitter environment.
Greg, you’re absolutely right. This was an oversight, rather than a design goal. I’ll update it in the next blog post when I discuss abstraction in the database layer. Thanks for pointing it out!
I’ve found it significantly better to lazy load utilities or helpers such as the HTTP object. For example, you can maintain the feel of using a “use once, throw away” object (I assume you meant the HTTPRequest and not the Twitter object) by having a setRequest and getRequest method on the Twitter class.
So, within the getRequest() method if there is not a HTTPRequest object already “injected” then go ahead and instantiate one. This makes it so easy to test in isolation with mocks as Greg mentioned without sacrificing the ease of use you have from your original code.
Daniel O'Connor (@clockwerx) wrote at 8/26/2009 7:11 pm:
PEAR’s HTTP_Request2, HTTP_Request2_Adapter_Mock and Log can solve your problems kinda neatly here – that way you can kill off all of this test() tomfoolery.
The mock adapter can be pre-armed with faked responses, making your class work exactly the same as it would in production, and all you do is swap the HTTP_Request2 driver from Mock to Real.
The Log object can provide you visibility; outputting to nowhere in production, and whereever you configure in testing.
Daniel, you make a lot of great points. Since the refactoring isn’t finished yet, I’ll be sure to adapt some of your thoughts into the future posts.
I’ll be dropping the “test” stuff in a future post, because testing should be abstracted into unit tests.
Ray Paseur (@Ray_Paseur) wrote at 8/28/2009 11:29 am:
While you can use file_get_contents() if CURL isn’t there, they are not the same things in at least one or two important ways. CURL will let your request time out. The file_get_contents() function does not do that. On my host, file_get_contents() adds its elapsed time to the script execution time with unpleasant results if the remote service is not available. Splat.
Ray, that’s a very good point. I think that making use of one of the PEAR classes, and/or making use of pecl_http would help resolve that issue and allow for the OOP structure we’re looking for.
| « FIEO: Filtering Input with PHP’s Filter Functions | My Essential WordPress Plugins » |