Class conflict, namespace collisions

maybe change main class name to DrupalCache or CRCache to predict future namespace collisions?

same problem :(
Any fix?

same problem...

суперфедя, решил проблему так: http://www.drupal.ru/node/39142#comment-207837

I have worked around this in three ways.

First, I renamed the Cache class to CacheEngine, and all the engines now extend this class.

Second, I renamed the $cache variable to $cacherouter.

Third, I changed require(engine) to require_once(engine) because I was getting errors about redeclaring a class.

I have submitted a patch to the latest version with these changes. They seem to work fine on my setup.

Yay! +1 to CacheEngine

+++ cacherouter/cacherouter.inc	2010-01-23 18:49:03.000000000 -0800
@@ -19,11 +19,11 @@
  */
 function cache_get($key, $table = 'cache') {
   global $cache, $user;
-  if (!isset($cache)) {
-    $cache = new CacheRouter();
+  if (!isset($cacherouter)) {
+    $cacherouter = new CacheRouter();

should be global $cacherouter, $user

I see no reason to change $cache to $cacherouter inside engines except parent class name - all this variables are local so there's no collisions

Yeah, I went a little overboard there. But I was not 100% sure how it was used when I made the change - but it's all good. I've done a lot more changes since then to allow for integration between dtools and cacherouter (ie. so the 'simple' performance storage can be saved using cache, even when not using APC as a backend).

This required the creation of a cache_delete standalone function, but also creation of a cache_list function (and equivalent support in the engines and cache router and cache interface). Which also required the creation of an unkey() function equivalent to key(). All in all, probably too much work, but it was an interesting experiment.

But just changing $cache -> $cacherouter is necessary to support phpbb3.

@PreZ this patch is more readable

I think we should take D7 approach and use Drupal{engine}Class for Engines this could help to prevent class name collisions in future

...and makes easy to maintain all branches

Take a closer look at D7 implementation I think it should be
DrupalCache{Engine}
or
Drupal{Engine}Cache

because of Drupal 7 core uses
DrupalDatabaseCache classname

This was fixed for DRUPAL-6--1 branch

I choose DrupalRouterCache for base class to be more compatible with D7
All Engine classes now {type}CacheEngine

Now it should be ported to 5 branch to proceed next RC

I don't think I like DrupalRouterCache as I think it is a confusing name. I think the default class should be CacheEngine and the rest of the engines should be {type}CacheEngine. DrupalRouterCache is confusing because the default class of the CacheRouter is called CacheRouter and I don't want any ambiguity between the two.

CacheEngine is an abstract class that is only really used like an interface not a normal Cache Engine and it certainly isn't the Router. It is simply a base class that we use some functionality of in the cache engines.

steve

Agreed with confusion. Maybe CacherouterEngine is better name for base (abstract) class name to prevent future collisions and looks more native for me.

If we talk about D7 implementation and CacheRouter class so name should be Drupal{type}Cache (like DrupalDatabaseCache) because of nature of cacherouter this class just a router so lets proceed with DrupalCacherouterCache for main class and CacherouterEngine for abstract class for engine?

Engines already {type}CacheEngine in D6 branch but we need decide on main class and engine abstraction

Thre reason I think CacheEngine is a good name is because it is the abstract class and not an actual _implementation_ of that class. So it won't conflict with the naming conventions used in DrupalCache. I don't think we have to follow core's naming conventions for our internal cache classes anyway, we just need to construct them that they won't interrupt or interfere with core.

I meen collisions when drupal used for integration with any third-party software like bb and others, so prefixing will help

Anyway here the patch, if we use CacheRouter as main class so abstract class should be CacheRouterEngine to unify naming

That sounds fine.

Commited to 6 branch

commited into 5 http://drupal.org/cvs?commit=324984 with code cleaning

Suppose fixed for D7 http://drupal.org/cvs?commit=325014

Next I think to rename global $cache_router to $_cache_router as proposed by http://drupal.org/coding-standards#naming

Here patches to review

Patches are against CVS HEAD or rc1?

DRUPAL-5 & DRUPAL-6--1 http://drupal.org/project/cacherouter/cvs-instructions