PHP RSS/XML reader basic class

Question

I write this class to process and output xml/rss feeds. I'm still working on, but any suggestion will be appreciated. As a little trick if the xml file come from an online resource, I've added a method to cache the xml file as a json file.

<?php class NewsFeed{ public $url; private $cachedFeed; private $cachedFile; private $feed; private $xml; private $json; public function __construct($url){ $this->url = $url; } public function showLiveFeed(){ $feed = $this->processFeed(); return json_decode($feed, true); } public function showCachedFeed(){ $cachedFeed = $this->loadCachedFile(); return json_decode($cachedFeed, true); } private function processFeed(){ $feed = $this->url; $xml = simplexml_load_file($feed, 'SimpleXMLElement', LIBXML_NOCDATA); $json = json_encode($xml); $cacheFeed = file_put_contents('cachedFeed.json', $json); return $json; } private function loadCachedFile(){ if(file_exists('cachedFeed.json')){ $cachedFile = file_get_contents('cachedFeed.json'); return $cachedFile; } } } ?> 

Answer 1

A caching strategy is not well thought. There is no clear scenario, whether a cached or a non-cached version should be used. Besides, a cache must have a timeout after which it is considered staled and must be renewed.

There is also a filename issue. With only a filename, which makes a relative path, you will end up with having a cachedFeed.json file in the every directory. The filename must be unique and certain, which means it must include an absolute path to the file.

Another possible issue is a case when 2 feeds are used on the same site. What would happen to a cache file in this case? A filename must be unique. the simplest solution would be just md5() from the feed url.

Other minor issues include excessive naming. your class already have the word Feed in its name, no need to duplicate it in the method names. Also, there are a lot of class variables that a never actually used.

So the final code could be like

<?php class NewsFeed { protected $url; protected $cacheFilename; protected $cacheTimeout = 3600; public function __construct($url) { $this->url = $url; $this->cacheFilename = $this->buildCacheFilename(); } public function get() { $json = $this->readCached(); if(!$json) { $json = $this->download(); } return json_decode($json, true); } protected function download() { $xml = simplexml_load_file($this->url, 'SimpleXMLElement', LIBXML_NOCDATA); $json = json_encode($xml); file_put_contents($this->cacheFilename, $json); return $json; } protected function readCached() { if (!file_exists($this->cacheFilename)) { return false; } if (filectime($this->cacheFilename) < (time() - $this->cacheTimeout)) { return false; } return file_get_contents($this->filename); } protected function buildCacheFilename() { return $_SERVER['DOCUMENT_ROOT'] . "../cache/feed" . md5($this->url); } } 

So we have just a single method get() to read the feed contents, which is first checking for the cached version and then reads the feed online if the cache is not available. A timeout is implemented to make sure that the cached data doesn't get too old and outdated.