Get the view class factory methods out of the view class itself

Hello,

I noticed some design errors in the view (view.inc file) class which could be easily fixed. I would like to help you to get a better design of this class, since it's the most important in your module.

You have some factory methods in this class which are, basically, static methods, I think you should implement them in another class, dedicated to the factory pattern implementation, these methods are (I might be missing some of them):

• function &load($arg, $reset = FALSE)
• function load_views()
• save()

The reason why I volontary put the save() method here is if your view objet has not the responsability of loading itself, then it would be quite logical that it does not have the responsability to save itself. This has other reasons than pure logic, by impleting both the load and save factory method in the same factory class, a internal static cache would be easier to handle (refresh or clear some pieces of the static cache when saving a view, for example).

This particular factory class could implemented in another class (i.e. view_factory for example), this would give you some advantages:

• Code in view.inc would be smaller, thus easier to read and navigate into (generally more understandable);
• By separating the factory from the object itself, you then could implement the factory as a singleton if you want;
• The factory could carry a lot of other factory methods without disturbing the view class itself

In all case, if you implement static factory method, please keep the "static" keyword before the function signature in the objets, by statically calling instance methods on objets, you will either generate PHP warning (if the method does not use the $this pseudo variable), either have PHP fatal errors (is the $this peudo variable is used in the local method context).

Notice that statically called methods were in the past (may be untrue anymore, since PHP evolve a lot, but it may be actually be true on a lot of servers which uses older versions) faster than running instance methods.

If you don't want to use the 'static' keyword because you are afraid to loose backward PHP 4 compatibilty, then you can implement the factory as a singleton, and instanciate the objet in a function of views.module which could be like this:

function views_get_factory() {
  static $factory;
  if (!isset($factory)) {
    $factory = new view_factory();
  }
  return $factory;
}

$my_view = views_get_factory()->load("my_view_name");

Which is the singleton pattern implementation using a procedural PHP function.