Notes in no particular order, most of them fairly minor/easy to fix:

* Typo in the Expiration definition. " This it calculated" should be "This is calculated". It looks like that is a typo in PSR-6, too, which we ought to fix. :-)

* " If a negative or zero TTL is provided, the item MUST be deleted from the cache if it exists, as it is expired already." - We discussed this a bit at phpWorld, though I don't recall the exact conclusion. I think we decided that this is already implicit in PSR-6; making it explicit here is probably unnecessary, but not harmful.

* In Cache Miss, make null a code snippet with `null`.

* The definition of Cache Hit appears to be missing. Is that deliberate, or an oversight?

* Is there a reason the "Data" section was not borrowed from PSR-6? I'm assuming all of the same logic applies to PSR-16, so it should either be duplicated like the definition section is or both should be omitted and refer to PSR-6 explicitly.

* Some docblock lines end in a period. Others don't. Please standardize on including the period universally.

* delete() should have a meaningful return, no? Even set() has one, and in PSR-6 it returns a boolean.

* Ibid for clear().

* getMultiple() accepts array|Traversble, but returns an array. I would have expected the other way around; accept an array of short strings, return a traversable of "stuff".

* array|Traversable should be array|\Traversable, or maybe iterable.

* On has(), "Identify if an item is in the cache" should probably be "Determines if an item is in the cache."

* I don't know that there's an official standard on verbage for docblocks, but I would encourage following the action-style. That is, read as though it begins with "this method..." Eg, "This method... Persists a set of key/value pairs"; "This method... deletes multiple cache items..."; etc. Currently the verb tenses are inconsistent.

* For increment/decrement, returning "value or false" is an anti-pattern. It's a PHP language anti-pattern, but still an anti-pattern. Don't do that. If something breaks, throw an exception because the data is now invalid anyway.

* And the only major point: I do not think the CounterInterface belongs here. There's plenty of use cases for an atomic counter that are not part of a cache; remember a cache could get wiped at any time and the application must continue to function. Sometimes wiping a counter is OK, other times it's totally not. That a cache is used for that is an implementation detail.

I would instead recommend splitting CounterInterface off to its own (small) PSR. I have no problem with small PSRs if they cover their target case adequately. Someone could certainly then implement both CounterInterface and SimpleCache\CacheInterface (or CounterInterface and CacheItemPoolInterface if they were so inclined) and lose nothing; or they could implement it on a persistent backend of some sort.

--Larry Garfield

On 11/16/2016 03:22 PM, Jordi Boggiano wrote:
Heya,

We believe PSR-16, Simple Cache, is now ready for final review. As
coordinator, I hereby open the mandatory review period prior to a formal
acceptance vote; voting will begin no earlier than December 1st, 2016.

Here are links to the most current version and its meta document:

https://github.com/php-fig/fig-standards/blob/1cf169c66747640c6bc7fb5097d84fbafcd00a0c/proposed/simplecache.md

https://github.com/php-fig/fig-standards/blob/1cf169c66747640c6bc7fb5097d84fbafcd00a0c/proposed/simplecache-meta.md


The package containing the interfaces is there:

https://github.com/php-fig/simplecache


The latest important changes to the interfaces can be found at:

https://github.com/php-fig/simplecache/releases/tag/0.2.0


And FWIW, Scrapbook already provides a PSR-16 implementation in its upcoming release: https://github.com/matthiasmullie/scrapbook/blob/master/src/Psr16/SimpleCache.php


Thanks for your time reviewing!

Cheers


--
You received this message because you are subscribed to the Google Groups "PHP 
Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to php-fig+unsubscr...@googlegroups.com.
To post to this group, send email to php-fig@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/php-fig/be14f4f9-5adf-b508-8907-34616b1df174%40garfieldtech.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to