2
\$\begingroup\$

I have a class that holds functions for caching using PHPFastCache. There are quite a few static functions in here and for each one I am declaring:

$cache = phpFastCache();

Is there a better way I can do this? It just feels wrong. Below is the full class maybe there are other changes I can do.

/**
 * Class Cache
 * Using PHPFastCache to cache data
 */
class Cache
{

    /**
     * put a data in to cache with a key, and time in minuets
     * @param $key
     * @param $value
     * @param int $minuets
     * @return mixed
     */
    public static function put($key, $value, $minuets = 1)
    {
        $cache = phpFastCache();
        return $cache->set($key, $value, $minuets * 60);
    }

    /**
     * Add value to Cache if does not already exist
     * @param $key
     * @param $value
     * @param int $minuets
     * @return bool
     */
    public static function add($key, $value, $minuets = 1)
    {
        if (self::has($key)) {
            return false;
        } else {
            self::put($key, $value, $minuets);
            return true;
        }
    }

    /**
     * Check if value is in Cache
     * @param $key
     * @return mixed
     */
    public static function has($key)
    {
        $cache = phpFastCache();
        return $cache->isExisting($key);
    }


    /**
     * Add a Key to the cache forever(25years)
     * @param $key
     * @param $value
     * @return mixed
     */
    public static function forever($key, $value)
    {
        return self::put($key, $value, 1 * 60 * 60 * 24 * 365 * 25);
    }

    /**
     * Get a value from cache and then remove from cache.
     * @param $key
     * @return bool
     */
    public static function pull($key)
    {
        $value = self::get($key);
        self::Remove($key);
        return $value;
    }

    /**
     * get value out of cache, ability to specify default if key is not in the cache
     * @param $key
     * @param bool $default
     * @return bool
     */
    public static function get($key, $default = False)
    {
        $cache = phpFastCache();
        if ($default !== false && self::has($key) === false) {
            return $default;
        } else {
            return $cache->get("$key");
        }
    }

    /**
     * remove key from cache
     * @param $key
     */
    public static function forget($key)
    {
        $cache = phpFastCache();
        $cache->delete($key);
    }

    /**
     * get information on key, such as time left in cache.
     * @param $key
     * @return mixed
     */
    public static function cacheInfo($key)
    {
        $cache = phpFastCache();
        return $cache->getInfo($key);
    }

    /**
     * remove everything from the cache
     */
    public static function clearCache()
    {
        $cache = phpFastCache();
        $cache->clean();
    }
\$\endgroup\$
2
\$\begingroup\$

I don't know how phpFastCache works, but normally I would use a constructor to get the cache once and then reuse the field in the methods. Something like this:

class Cache
{
    private $cache = NULL;

    public function __construct()
    {
      $this->cache = phpFastCache();
      // error checking?
      if (!isset($this->cache)) throw new Exception('Could not get a cache.');
    }

    public static function put($key, $value, $minuets = 1)
    {
        return $this->cache->set($key, $value, $minuets * 60);
    }

    public static function get($key, $default = False)
    {
        if ($default !== false && self::has($key) === false) {
            return $default;
        } else {
            return $this->cache->get("$key");
        }
    }

    public static function forget($key)
    {
        $this->cache->delete($key);
    }

}

I have noticed that your class doesn't add much functionality. Is that still to come? There's no point wrapping a perfectly good class and only copy, or perhaps limit, its functionality.

\$\endgroup\$
  • \$\begingroup\$ I'd rather throw an exception then to just stop all code execution like that. \$\endgroup\$ – Kid Diamond Apr 16 '15 at 9:42
  • \$\begingroup\$ @Kid Diamond: I agree, and changed it. It was just meant as a simple example, but I had better give the correct example. \$\endgroup\$ – KIKO Software Apr 16 '15 at 11:21
4
\$\begingroup\$

Probably the proper way would be to pass it via constructor as this would allow you to choose which driver phpFastCache() should without interfering with your class. And thats commonly referred as dependency injection. You have to type hint on the phpFastCache() returned instance interface which is phpfastcache_driver. This gives you two things, you can easily change underlying caching mechanism and still lock only to the instances that you know that implement methods you use in your class.

Then the constructor would look like this:

public function __construct(phpfastcache_driver $cache)
{
  $this->cache = $cache;
}

And the initialisation of the class as follows:

$myCache = new Cache(phpFastCache('files'));

or

$myCache = new Cache(phpFastCache('memcached'));

or whatever storage engine phpFastCache supports.

Edit: I've noticed that you have static methods which do access non static property, you can not do that. Remove static from them.

\$\endgroup\$
  • \$\begingroup\$ Welcome ghispi. Staring with an answer +1. I hope you enjoy the site. \$\endgroup\$ – Legato Apr 21 '15 at 13:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.