A proposal for an extensible object oriented way for the page controllers

(Not sure if this is the right group for this topic)

I want to propose what I think it’s a better way for the page controllers, that I have used for a MVC implementation in a couple of (non Elgg) sites we created, using a more object oriented approach. This can be implemented right now without any change in the current Elgg engine, although it might have some optimization in future versions.

Instead of using one controller for every plugin page, we used only a controller class for the whole plugin. The plugin has a main function where it does the common tasks for all pages and call the right page rendering function depending on the received arguments. Those common tasks are now unnecesary repeated in every page controller and many times the same thing is done in different ways, even inside the same plugin.

Take for example an hypothetical plugin for news with two pages, one for edit and one for display:

class Controller extends BaseController {

   public function run($args) {

     // Common tasks like authorization, page owner, etc.

     // (it might be also in the BaseController class)

 

     $page = array_shift($args);

     switch ($page) {

        case ‘edit’:

           $this->runEdit($args); break;

        case ‘display’:

        default:

           $this->runDisplay($args); break;

     }

 

     if ($this->draw) page_draw($this->title, $this->body);

   }

 

   protected function runEdit($args) {

     $this->title = ‘News Edit’;

     $this->body = elgg_view(‘edit’);

     $this->draw = true;

   }

 

   protected function runDisplay($args) {

     $this->title = ‘News’;

     $this->body = elgg_view(‘display’);

     $this->draw = true;

   }

}

 

That’s it, the current plugin page handler can instantiate the class and pass it the url arguments instead of calling single page controllers. This would also eliminate the /pg/ vs. /mod/ issues because everything would be centralized in the same place.

But, another step forward. When you implement your own theme, you should edit your own views and you shouldn’t need to touch the controllers. Ok, maybe in an ideal world, but that thing doesn’t exist :)

For example, in our Elgg site, we had to edit every controller for adding a breadcrumb, but many other cases might exist, why don’t allow to extend the controller when you need to do it. (I know breadcrumbs might be implemented in other ways, but it’s only an example)

How to do it easily? Adding also a custom controller class for every plugin, and the page handler will always call this custom class. It might be an empty class, for example…

class CustomController extends Controller {

}

 Or, it might reproduce every method, calling to the parent methods for better clarity…

class CustomController extends Controller {

   public function run($args) {

     parent::run($args);

   }

 

   protected function runEdit($args) {

     parent::runEdit($args);

   }

 

   protected function runDisplay($args) {

     parent::runDisplay($args);

   }

}

Then, if you need to add some functionality, you can put your code before or after calling the parent function, or don’t call it at all and implement your own code for the whole function.

In this way, the original code remains untouched and it’s very clear in the custom class what functions you overrrided, if you do so.

It’s only a base for discussion. In future Elgg versions, the main page handler might instantiated the controller directly, without need a plugin page handler and many functions (or even all of them) currently in start.php (like page menus) could be moved to the controller class, making it easily overridable. Current plugins can do a ‘raw’ move to this way simply by copying every page controller into a method of the plugin controller class.

I uploaded here a modified version of Messages plugin for demonstrating the concept:
http://community.elgg.org/pg/plugins/morgar/read/391040/modified-messages-plugin-using-an-extensible-object-oriented-plugin-controller

It's only a test, please don't use it in a production site because it was not fully tested.

I would want to know your thoughts about it.

Regards

  • Hi Morger, This is good thinking, thanks.  Having looked at how "Social Engine" does some things, I think there is a lot that can be done better in elgg.  I just wish I could do all the things I have done so far with elgg in Social Engine, which is very clean, all themes are validated, code is beautifull laid out and clean... elgg has a way to go yet... and these kinds of ideas could make a real differance. Thanks.

  • Thanks Mark. Yes, I agree Elgg has many areas that can be improved, both engine and interface but specially interface, as we talked in the other thread. I hadn't seen Social Engine in a long time and yes, it looks really clean and polished and it looks like you can use it as is, and create a new site in short time, we can't do that with Elgg basic template.

    About my proposal, I adapted it from other scripts I have used. I think Elgg should move in a more 'object oriented' way, it would make easier to customize. Maybe it would require a bit more programming background in developers, but sure it will be more powerful.

    (sorry about my english)

    Regards

  • Hi Morgar, Don't aplolgise for your English, it is perfect.  I am not a programmer, so I only vaguely understand about OOP, but I do know it is the way ahead, and if it makes customising easier then this must be the goal.  I have looked at some of the Social Engine code, and I have to say it is easier to understand, and seems much more simple than elgg stuff.  is it OOP? sample below:

    <?
    $page = "event";
    include "header.php";

    if(isset($_GET['event_id'])) { $event_id = $_GET['event_id']; } else { $event_id = 0; }

    // DISPLAY ERROR PAGE IF USER IS NOT LOGGED IN AND ADMIN SETTING REQUIRES REGISTRATION
    if($user->user_exists == 0 && $setting[setting_permission_event] == 0) {
      $page = "error";
      $smarty->assign('error_header', 639);
      $smarty->assign('error_message', 656);
      $smarty->assign('error_submit', 641);
      include "footer.php";
    }

    // DISPLAY ERROR PAGE IF NO OWNER
    $event = new se_event($user->user_info[user_id], $event_id);
    if($event->event_exists == 0) {
      $page = "error";
      $smarty->assign('error_header', 639);
      $smarty->assign('error_message', 828);
      $smarty->assign('error_submit', 641);
      include "footer.php";
    }



    // GET PRIVACY LEVEL
    $privacy_max = $event->event_privacy_max($user);
    $is_event_private = 0;
    if(!($privacy_max & $event->event_info[event_privacy])) { $is_event_private = 1; }

    // UPDATE EVENT VIEWS IF EVENT VISIBLE
    if($is_event_private == 0) {
      $event_views = $event->event_info[event_views]+1;
      $database->database_query("UPDATE se_events SET event_views='$event_views' WHERE event_id='".$event->event_info[event_id]."'");
    }

    // GET EVENT LEADER INFO
    $eventowner_info = $database->database_fetch_assoc($database->database_query("SELECT user_id, user_username FROM se_users WHERE user_id='".$event->event_info[event_user_id]."'"));

    // GET EVENT CATEGORY
    $event_category = "";
    $parent_category = "";
    $event_category_query = $database->database_query("SELECT eventcat_id, eventcat_title, eventcat_dependency FROM se_eventcats WHERE eventcat_id='".$event->event_info[event_eventcat_id]."' LIMIT 1");
    if($database->database_num_rows($event_category_query) == 1) {
      $event_category_info = $database->database_fetch_assoc($event_category_query);
      $event_category = $event_category_info[eventcat_title];
      if($event_category_info[eventcat_dependency] != 0) {
        $parent_category = $database->database_fetch_assoc($database->database_query("SELECT eventcat_id, eventcat_title FROM se_eventcats WHERE eventcat_id='".$event_category_info[eventcat_dependency]."' LIMIT 1"));
      }
    }


    // GET EVENT COMMENTS
    $comment = new se_comment('event', 'event_id', $event->event_info[event_id]);
    $total_comments = $comment->comment_total();


    // GET INVITED MEMBERS
    $where = "(se_eventmembers.eventmember_status<>'-1')";
    $total_invited = $event->event_member_total($where);
    $members_invited = $event->event_member_list(0, 5, "RAND()", $where);

    // GET GUESTS WHO HAVEN'T RESPONDED YET
    $where = "(se_eventmembers.eventmember_status='0')";
    $total_mia = $event->event_member_total($where);
    $members_mia = $event->event_member_list(0, 3, "RAND()", $where);

    // GET ATTENDING GUESTS
    $where = "(se_eventmembers.eventmember_status='1')";
    $total_attending = $event->event_member_total($where);
    $members_attending = $event->event_member_list(0, 5, "RAND()", $where);

    // GET "MAYBE" GUESTS
    $where = "(se_eventmembers.eventmember_status='2')";
    $total_maybe = $event->event_member_total($where);
    $members_maybe = $event->event_member_list(0, 3, "RAND()", $where);

    // GET GUESTS NOT ATTENDING
    $where = "(se_eventmembers.eventmember_status='3')";
    $total_notattending = $event->event_member_total($where);
    $members_notattending = $event->event_member_list(0, 3, "RAND()", $where);


    // CHECK IF USER IS ALLOWED TO COMMENT
    $allowed_to_comment = 1;
    if(!($privacy_max & $event->event_info[event_comments])) { $allowed_to_comment = 0; }


    // GET CUSTOM EVENT STYLE IF ALLOWED
    if($event->eventowner_level_info[level_event_style] != 0 & $is_event_private == 0) {
      $eventstyle_info = $database->database_fetch_assoc($database->database_query("SELECT eventstyle_css FROM se_eventstyles WHERE eventstyle_event_id='".$event->event_info[event_id]."' LIMIT 1"));
      $global_css = $eventstyle_info[eventstyle_css];
    }

  • morgar - reminds me of how cakephp does controllers. I think codeigniter (and I'm sure many other frameworks) does something similar.

    What are you expecting to happen by posting this here? Are you trying to encourage other plugin authors to use this pattern? Do you want someone to develop a helper plugin that makes it easier to develop page handlers in this manner? Are you hoping to gain the attention of the core developers? I'm curious.

  • @Mark Bridges: without having seen the code in detail I might say it's an extrange mix. Because there is some object oriented code close to database code that seems that they shouldn't be there. What appears as 'smarty' is a widely know template engine. I don't like very much the Smarty approach, I prefer the way of Elgg in this matter.

  • @Cash: I know about CodeIgniter and CakePhp but I didn't see how they solve the controllers, I will take a look. I adapted the idea from other scripts I have used like AjaxChat, that have that extensibility approach, although not specifically in the form of a controller.

    About your question, English is not my native language and I'm not sure what is your point. I'm posting this proposal here because I think it might make easier the customization for everybody (including me) if controllers would be done in this way.

    And I would like other developers express their opinion about it and might point the flaws that it could have. For our main Elgg site http://www.shertonenglish.com/social/ I had to edit almost every page controller and I found the same thing done in some many different ways, even inside the same plugin.

    Using this object oriented way and moving repeated code to base clases would have as result a cleaner and more legible code for everyone. And it might impose some rules of order, like don't mix /mod/ and /pg/ urls.

    Again, not sure about your point behind "hoping to gain the attention of the core developers". Yes in the sense that they could evaluate this proposal and adopt it if they think it's valuable. I change a couple of words with Brett about this and he said he liked the concept and he think something like this be a great addition to Elgg in 1.8 or 1.9.

    Sorry about my english and sorry is I misunderstood something.

    Regards

  • Brett has already created a recommendation on directly calling /mod/ here: http://docs.elgg.org/wiki/Actions_PageHandlers_DirectAccess

    I agree that there are some advantages to using classes for page controllers - code resue being the primary advantage.

    A next step could be to produce a plugin that provides a base class for page handlers. That might encourage plugin authors to use this approach.

     

  • @Cash: Sorry, I just realized that when you said about posting this proposal "here" you mean, in this group about "Elgg's Roadmap". Got it. I was not sure what would the right place for it.

    Yes, I think using clases (and a base class) for plugins might make it easier for both plugin developers and for the other developers that will use -and maybe extend- them in their projects.

    In a similar way of pages, actions could be also moved to the controller, adding a second entry point like a runAction() function. In doing so, all the plugin logic will be encapsulated in the controller.

    The only doubt I could have is how familiarized is the average php developer with the OOP concepts.

  • @morgar I don't think we should worry about if the plugin developer understands OOP...if he doesn't and wants to program in Elgg, he's got to learn.  Elgg isn't completely OOP-based, but the datamodel is. 

    If this idea can be fleshed out into a helper plugin as Cash suggests it might be something worthwhile to investigate for inclusion in a later release.  It would be a nice loosely-coupled way to offer this functionality to developers who wish to use it.

  • Sorry to dig this topic up, but I think you'll find this will slightly reduce performance without adding flexibility. What this does is move (not replace) the routing code into a separate file, and move page code from procedural files to class files. The net effect is adding a file include, and introducing more (and slower) method calls.

    Although Elgg's routing doesn't use objects, it's fast and flexible enough. I think it's an area (and there are a lot of these) where introducing classes isn't going to improve much. I think the big gains (w/r/t plugins) will be from dicing up as much plugin functionality into component classes that can be autoloaded (and making sure Elgg supports class method calls for hook handlers, etc.). Only after we have several plugins like this will we start to recognize patterns of components that could be refactored out into generic components that could be shipped with Elgg.

Feedback and Planning

Feedback and Planning

Discussions about the past, present, and future of Elgg and this community site.