Rename CachedObjectStorage as Collection/Cells and refactor to optimize

[MarkBaker]

Rename CachedObjectStorage as Collection/Cells and refactor to optimize

[PowerKiKi]

@MarkBaker as I am sure you are aware PSR-16 Simple Cache is about to be published. I was wondering if that would make sense for PhpSpreadsheet to rely on that standard, instead of re-implementing so many adapter ourselves. What do you think ?

Also I noticed that igbinary is implemented as a cache itself, whereas I would think it's only a required step for every kind of cache. I mean we should be able to use, eg, memcache+igbinary, or sqlite+igbinary.

[titanrat]

@PowerKiKi PSR-16 is not yet published as i know. If phpSpreadsheets 1.0 release is coming soon - it isn't nessesary to do this refactor right now. We can still use adapters.

[PowerKiKi]

Actually PSR-16 was accepted 3 weeks ago. While it clearly still is a very young PSR, I feel it fits exactly our need. And PhpSpreadsheet is not quite ready for a release anyway. So I'd really like to consider this option.

[Quix0r]

PSR-16 looks to young to me. InvalidArgumentException is not an interface in SPL,, it is a class: http://php.net/manual/en/class.invalidargumentexception.php Well, I know, not the right place here. But you should reconsider taking it as it may break more things (because of naming conflicts with well-establisched code like the SPL is) as it solves.

[PowerKiKi]

Are you saying you think \Psr\SimpleCache\InvalidArgumentException from PSR-16 may conflict with \InvalidArgumentException from SPL ? If that's so, I don't think there any issue since PSR-16 is properly namespaced.

Did you have other argument that would show why it's too early ? or what kind of improvement you would expect to make it a suitable choice ?

[Quix0r]

Well, if PSR has never heard of well-established naming conventions (like the Java naming convention is), then have it their way. Think of, why PHP is getting finger-pointed, because not because of PHP itself is bad but the people doing stuff around it are bad.

Yes, and it conflicts, if you only see InvalidArgumentException (the class name itself without namespace), you will mostly think about the SPL class as this name hints it But you will never think of that it is an interface. That is why well-established naming conventions exists like the Java naming convention.

That is why I was very happy with the naming scheme in PhpExcel and not here in PhpSpreadsheet. Currently, I keep aliasing them so in source code I can simply see that they are from PhpSpreadsheet and not generic (SPL).

Causing lesser confusion is better for everyone. Newbies can get easily into the code. And don't think of ignoring use (some kind of "import" as the other class/interface is being imported into the current namespace), it has it's reasons of not having everywhere a namespace around.

[PowerKiKi]

You seem to be arguing against namespace as they exists in PHP. While it may be an interesting debate, it's a much bigger topic and out of place here.

So far I didn't see any practical argument in favor or against PSR-16.

[Quix0r]

No, I recommend them in addition to better class names. :-) But okay, getting "a little" OT.

[PowerKiKi]

So after a some analysis, here is my approximate plan for the refactoring:

Class	Plan
CacheBase	refactored as wrapper Cells around a PSR-16 instance, with PhpSpreadsheet specifics
Memory	kept, the only implementation that actually lives in PhpSpreadsheet
APC	PSR-16 via php-cache.com (and APCu)
Memcache	PSR-16 via php-cache.com
SQLite3	PSR-16 via php-cache.com
Wincache	PSR-16 via php-cache.com
ICache	dropped, it's essentially PSR-16
DiscISAM	dropped, planned in php-cache.com
PHPTemp	dropped, future implementation possible in php-cache.com
Igbinary	dropped, PSR-16 works with objects directly and is responsible for serialization
MemoryGZip	dropped, IMHO this is a half-baked solution, should use a real cache to really free up memory
MemorySerialized	dropped, IMHO this is a half-baked solution, should use a real cache to really free up memory

Classes marked as dropped do not mean that the feature will be gone forever. It just means that there will be no solution out of the box as far as I know. But it is possible for a third-party to re-implement those, either via php-cache project, or as a standalone PSR-16 implementation.

By adopting PSR-16 we automatically get new cache supported, such as memcached, redis, mongodb amongst others (~50 projects using PSR-16 so far). It will also drastically simplify our codebase. Making it easier to test and delegating cache implementation to third-parties so we can focus on our core features.

@MarkBaker, would you have any opinion to share about this plan ?

[titanrat]

@PowerKiKi So. #75 May be closed?