How keeping code in the right layer saved a website

At my job at e-sites, one of the projects I worked on was the new Sport 1 website. The website went live yesterday morning. About 20 minutes later, I was asked to look at why it was so slow.

For the website, we had a layered setup. In our ‘domain layer’, we had models and entities, where a model understood how to work with an entity. So, say I want all the news articles for a specific sport, I can do this:

$aNewsArticles = $oNewsModel->getForSport($iSportId);

Where $oNewsModel is an instance of our news model. After the call, $aNewsArticles would be an array, containing instances of our news entity class. Simple, right?

Well, apparently not. Since I setup the groundwork for the project and (basically) then went on vacation with little time to explain it to others, this is what I ran into:

Class NewsModel {
	public function getForSport($iSportId) {
	    $aArticles = query("SELECT newsId from news where sportId = %d", $iSportId);
	    $aArticleList = array()
	    foreach($aArticles AS $aArticle) {
	        $aArticleList[] = new NewsEntity($aArticle['newsId']);
	    }
	    return $aArticleList;
	}
}

Do you see where this is going? Yes, thats where

Class NewsEntity {
	public function __construct($iArticleId) {
	    $aData = query("SELECT * from news where newsId = %d", $iSportId);
	    $this->iId = $aData['newsId'];
	    $this->sTitle = $aData['newsTitle'];
	    /* ..... */
	}
}

Taking into account that on the frontpage we a bunch of lists with a total of 80 articles that where all retreived the same way, we where executing at least 80 queries to much. And all that, just because somebody put the query in the wrong place.

In the end, it was a fairly easy fix, but it shouldn’t have been done this way in the first place. Now, I understand how this would have happened. Somebody had to do the first thing with news and, in that particular case, knew the ID of the news-item, since it was part of the URL. Well, if you already know the ID, whats easier then this:

$oNewsItem = new NewsEntity($iId);

Yea, that must be the easiest way to make that work. Feed the ID into the entity object and have that get the rest. And then, when you need a bunch of those objects, well, just selecting the IDs of the items you need and creating new objects, which then get the rest of the data, thats pretty easy, too. To bad its also not so fun for the database server.

So now we know how it happened, how do we make sure it doesn’t happen again? Obviously, making sure everybody has the same idea of what should go where is a (good) start. Ideally, there would be time to write some documentation, and you’d be able to oversee (and participate in) the further development, meaning you can put a stop to something while its happening instead of after the website went live. But, like I said, I was going on vacation, so I did neither. 🙂