matthiasmullie / scrapbook

PHP cache library, with adapters for e.g. Memcached, Redis, Couchbase, APC(u), SQL and additional capabilities (e.g. transactions, stampede protection) built on top.
https://www.scrapbook.cash
MIT License
315 stars 27 forks source link

Minor issue in SimpleCache::ttl() comment #31

Closed bkdotcom closed 6 years ago

bkdotcom commented 6 years ago

Sorry, I didn't notice this when with my prev SimpleCache::ttl() issue

https://github.com/matthiasmullie/scrapbook/blob/master/src/Psr16/SimpleCache.php#L235-L237

/*
 * PSR-16 specifies that if `0` is provided, it must be treated as
 * expired, whereas KeyValueStore will interpret 0 to mean "expires
 * this second, but not at this exact time" (could vary a few ms).
 */

KeyValueStore interprets 0 as no-expiration, not expire-now

Also... PSR-16 doesn't specify, but I'm wondering if it'd make sense to throw InvalidArgumentException rather than \TypeError for invalid TTL ?

Ie, the specification only documents

/**
 * @throws \Psr\SimpleCache\InvalidArgumentException
 */

nothing about potentially throwing some other exception.

matthiasmullie commented 6 years ago

Ugh - dealing with all these different implementation differences seems to have melt my brains when writing that comment. Fixed it - thanks!

As for how invalid input is handled: it was never explicitly addressed in the PSR how it should be done so we could indeed chose. Some have attempted to formalize how to deal with such invalid input, but there was insufficient interest to push that through. The proposal at that time (and what a couple of other libraries do) was to throw a TypeError, rather than an InvalidArgumentException. So even though it hasn't officially been defined how this should be dealt with, TypeError seems to be the most commonly accepted way.