Re: Proposal for new package

Jeff Trudeau wrote: > To clarify, this library requires pecl_memcache in order to interface > with memcached. It is written in PHP (not C), and thus can't > communicate natively with memcached. > > I have attached the source and API docs, and a small test script. > Please let me know if you think this library would make sense as a PEAR > package. > > Thanks! Jeff, I still don't see the major advantage of the class over straight memcache usage, would care explaining a bit more. I did a cursory review of the code you attached (btw: next time please put it online to save everybody time) and it looks like basically a wrapper that doesn't provide much more than calling save() whenever needed (which might be convenient though.) On the code itself, CacheFactory class is not needed, put the getInstance() in CacheSet class instead, and make the CacheSet::__construct() protected or private. Use __get() and __set() for CacheSet accessor methods instead of get() and set(). I don't know the HashSet Java API but I have a gut feeling you might have copied it too closely and not taken advantage of PHP-specificities. For instance why not implementing SPL ArrayObject, __clone() __set(), __get()? And, don't use global variables, unless following the strict PEAR standards for naming (but I understand that it is not a PEAR proposal at this stage.) I didn't see any exception either, especially in the factory / construstor. Might be useful to throw some there in connection / settings issues. If you implement (some of) the suggestions above, you'd be a much better shape to propose it to PEAR, but that's just my opinion :-) -Philippe