| « Avoiding Notices: When to Use isset() and empty() | The Slow Death of PHP 4 » |
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.
The topics discussed in this entry may be fairly advanced. Please feel free to ask questions, and discuss best practices.
If you’ve been following this series from the beginning, take a moment to look at the original code sample and compare it with where we are now. We’ve come a long way!
There is one last area that I want to address, and this has everything to do with object-oriented principles and code reusability. For those who are familiar with OO programming, they realize that the use of classes does not make something object oriented by nature. In this final part of the series, we’ll move one step closer to being object-oriented, by introducing the concepts of request and response objects.
At the moment, our object takes arguments like most functions do. This has some limitations. The first limitation is that the object must be aware: that is, it must have an understanding of the request it is being passed, and the response that it is getting from Twitter, as well as the response it will give back to our application. This means that in the event that something ever changes about the way that response is organized, we have to change this code, explicitly. I would like to avoid that.
The first way we will change this need to be aware is by altering the way the request comes into the object. We will do this by passing in a request object.
/**
* Constructor.
* @param HTTPRequest $http The HTTP request object.
* @param IRequest $request The request object.
* @param string $username The username of the Twitter user
* @param string $password The password of the Twitter user
* @return void
*/
public function __construct(HTTPRequest $http, IRequest $request, $username, $password)
{
$this->user = $username;
$this->pass = $password;
$this->http = $http;
$this->request = $request;
}
The request object will contain the message we plan to pass to the Twitter API, and could contain other information. It could potentially contain the username and password, but since the point of this object is to log in from the backend, I’m going to assume that the credentials are coming from a configuration file and not from the environment or the end user.
Why would we want to use a request object? The reason is that the API will be the same for the request object, regardless of the structure of the $_POST/$_GET/$_REQUEST superglobal arrays. Additionally, you can add filtering capabilities to your request object, and there are a number of robust features you can add (for example, automatically turning JSON into a request object for use with web services). Your object can be totally ignorant of the structure of the request, and still access it, through the settings provided in the request object.
The second thing that we want to do is apply this principle to our response. Our tweet() method returns true or false, but it returns true or false in error conditions as well as in success conditions. Additionally, there’s no ability for us to utilize the data that Twitter gives us back during a success or failure condition. This is sub-optimal.
Let’s assume that we have a response object capable of returning a response, and it can take a string, an exception, HTML or JSON (depending on which arguments you pass it). With a little modification, our code now looks like this:
/**
* A public method to post a tweet.
* @return object An object implementing IResponse.
*/
public function tweet()
{
$message = $this->request->get('message');
// Sanity checks. These are error conditions.
try {
$chars = strlen($message);
if(empty($chars) || $chars > 140)
{
throw new Exception('No message provided is not valid for tweeting due to a length violation. Max length: 140 chars. Min length: 1 char. Given length: ' . $chars);
}
if ($this->already_tweeted($message))
{
throw new Exception('This message has already been sent to Twitter');
}
} catch (Exception $e) {
$response = new ExceptionResponse($e);
return $response;
}
// STATUS UPDATE ON TWITTER
$this->http->setHost($this->baseHost . "statuses/update.json?status=" . $message);
$this->http->setCredentials($this->user, $this->pass);
$this->http->setOption(HTTPRequest::RETURN_TRANSFER, true);
$this->http->setOption(HTTPRequest::POST, true);
$TwitterResponse = $this->http->execute();
$rawResponse = $TwitterResponse->getRaw();
$array = json_decode($rawResponse, true);
// We are going to return a JSON-based response regardless of success
// or failure. The application calling this will determine whether or
// not we were successful.
$response = new TwitterResponse($rawResponse);
if(isset($array['id']) AND $array['id'] > 0)
{
$this->logTweet($message); // returns true.
}
return $response;
}
We’re doing several new and better things in there. First, we’re making use of exceptions for three possible error conditions: the condition where we have already tweeted this message, the condition where the message is too short for Twitter, or the condition where the message is too long for Twitter. In the event that one of these conditions exists, we throw an exception, which is caught, and then we return an ExceptionResponse object containing the Exception to help keep the script flowing (not posting to Twitter shouldn’t be a fatal error). In the event of actually attempting a posting on Twitter, the script returns a TwitterResponse object, which contains the information we got back from Twitter.
By having a standardized operation for response objects, we can predict what methods will be available and what information can be gleaned from the response object. For example, we can implement a getMessage() method in the ExceptionResponse object, and know that it will always contain data that it gets from Exception’s getMessage() method. We can re-implement this same getMessage() method in TwitterResponse and get back a valid message. This makes implementing our code easier.
In Conclusion…
Whew. Well that’s it. We’ve been through the entire series! If you didn’t heed my suggestion earlier to take a look at the original code and compare, feel free to do it now (the final version of the code is at the bottom of this blog entry). We’ve done a lot of refactoring: we examined the coding standards, worked out abstraction problems (twice!), fixed the broken business logic, implemented some unit testing while fixing the architecture, and finally implemented response and request objects. That’s a lot of work! It’s been an exciting journey, and looking at the finished product, we have a much better code sample than when we started.
Take an opportunity to pat yourself on the back for getting through the series, and then feel free to leave comments for those who will come after you saying what else you might add to this process. The process of code revision and refactoring is a long, arduous and sometimes contentious process, and it is not without it’s struggles and politics. But it is one that we can all do better at, and something we should do more often. Cheers!
/**
* Twitter Class
* @author Anonymous
*/
class Twitter
{
/**
* @var HTTPRequest HTTPRequest Object.
*/
protected $http;
/**
* @var object The request object/
*/
protected $request;
/**
* @var string The username for the Twitter account.
*/
protected $user;
/**
* @var string The password for the Twitter account.
*/
protected $pass;
/**
* @var string The base host for tweeting
*/
$baseHost = 'https://www.twitter.com/';
/**
* Constructor.
* @param HTTPRequest $http The HTTP request object.
* @param IRequest $request The request object.
* @param string $username The username of the Twitter user
* @param string $password The password of the Twitter user
* @return void
*/
public function __construct(HTTPRequest $http, IRequest $request $username, $password)
{
$this->user = $username;
$this->pass = $password;
$this->http = $http;
$this->request = $request;
}
/**
* A destructor.
* @return void
*/
public function __destruct()
{
unset($this->http);
}
/**
* Check to see if this tweet has already been posted.
* @param string $message The tweet
* @return bool
*/
protected function checkTweet($message)
{
$hash = md5( date('Y-m-d') . $message );
$c = new Criteria();
$c->add(TwitterLogPeer::THASH, $hash);
$count = TwitterLogPeer::doCount($c);
if($count > 0)
{
return TRUE;
}
return FALSE;
}
/**
* Log the tweet if it has already been tweeted.
* @param string $message The tweet
* @return bool
*/
protected function logTweet($message)
{
$hash = md5( date('Y-m-d') . $message );
try {
$logTweet = new TwitterLog();
$logTweet->setDate(time()); // This will be turned into a DateTime object.
$logTweet->setHash($hash);
$logTweet->setTweet($message);
$logTweet->save();
} catch (Exception $e) {
throw new Exception('There was an error in saving the item to the database.');
}
return TRUE;
}
/**
* A public method to post a tweet.
* @return object An object implementing IResponse.
*/
public function tweet()
{
$message = $this->request->get('message');
// Sanity checks. These are error conditions.
try {
$chars = strlen($message);
if(empty($chars) || $chars > 140)
{
throw new Exception('No message provided is not valid for tweeting due to a length violation. Max length: 140 chars. Min length: 1 char. Given length: ' . $chars);
}
if ($this->already_tweeted($message))
{
throw new Exception('This message has already been sent to Twitter');
}
} catch (Exception $e) {
$response = new ExceptionResponse($e);
return $response;
}
// STATUS UPDATE ON TWITTER
$this->http->setHost($this->baseHost . "statuses/update.json?status=" . $message);
$this->http->setCredentials($this->user, $this->pass);
$this->http->setOption(HTTPRequest::RETURN_TRANSFER, true);
$this->http->setOption(HTTPRequest::POST, true);
$TwitterResponse = $this->http->execute();
$rawResponse = $TwitterResponse->getRaw();
$array = json_decode($rawResponse, true);
// We are going to return a JSON-based response regardless of success
// or failure. The application calling this will determine whether or
// not we were successful.
$response = new TwitterResponse($rawResponse);
if(isset($array['id']) AND $array['id'] > 0)
{
$this->logTweet($message); // returns true.
}
return $response;
}
}
Brandon Savage is the author of Mastering Object Oriented PHP and Practical Design Patterns in PHP
Posted on 9/21/2009 at 1:00 am
Zilvinas wrote at 9/21/2009 5:30 am:
You’re creating a TwitterLog instance every time you log something. TwitterLog should be injected to improve testability. TwitterLog should be mocked out to allow the class to be tested in isolation from other components.
Brandon Savage (@brandonsavage) wrote at 9/21/2009 8:02 am:
I would normally have injected all dependent objects, but in this case the way Propel (the ORM I’m using) works it makes use of static methods. I could potentially move the business logic into the model, but as written it would be an impossible task to replace the static methods I’m using with dynamic ones that could be mocked out.
That said, you can have a custom configuration, so theoretically I could create a SQLite database in memory and test using that. Propel uses a Singleton, for better or worse, making a memory-based database perfect for testing.
Fixing up databases for unit tests is slow. You want your tests to execute in 2 secs so you could run them every time you press ctrl+s and you may have hundreds of those tests. And worse you may have queries that are not compatible with sqlite like MySQL date/time functions. And worse you would be testing logging and tweeting and if your test fails for logging you are not exactly sure what’s not working – logging or tweeting. Meaning it’s no longer a unit test and rather a functional test.
Wouldn’t say propel is doing a great job with singletons :)
*functional => integration
Diogo (@garotosopa) wrote at 9/21/2009 8:30 am:
Why exactly are you returning ExceptionResponse instead of just throwing an exception?
Diogo, the reason that I’m returning ExceptionResponse is because I’m assuming that we want to display some sort of message to the end user. I’m also assuming that this class is part of a controller, which will send its output to a response formatter.
Remember that an unhanded exception will stop the processing of an application. This would result in a white screen of death. Instead, I’d want to log the exception and display something to the user.
There are additional ways to handle the exceptions; you could wrap the tweet() method call in a try-catch block in another class, etc. There’s no one single “right” way to do it in this case.
Good question!
| « Avoiding Notices: When to Use isset() and empty() | The Slow Death of PHP 4 » |