SitePoint Sponsor

User Tag List

Results 1 to 19 of 19
  1. #1
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Arrow Improving Dispatcher design

    Hello...

    A couple weeks ago I "inherited" a dispatcher class witch was based on this article ("An introduction to the Model-View-Controller Pattern").
    My mission: I have to improve the design of this class.
    Reasons:
    -> use php 5 advantages
    -> the code started to smell when I tryed to add new features (eg. authentication part).
    Here is what I have right now:
    PHP Code:
    class Dispatcher
    {
        
    /** URL parts */
        
    private $parts;
        
    /** Config */
        
    private $config;
        
    /** Auth */
        
    private $auth;
        
        
    /**
         * Constructor
         * @access public
         */
        
    public function __construct() {
            
    // this will parse the URL parts
            // (index.php/__CLASS__/__METHOD__/__ADDITIONAL_INFO__/ )
            
    $this->parts  System::getParts();
            
    // general configs
            
    $this->config Config::instance();
            
    // authentication class
            
    $this->auth  = new Auth();
        } 
        
        
    /**
         * main switcher method
         * @access public
         * @throw Exception if the ENTRY_POINT is wrong
         * @return void
         * @TODO: @FIXME: REVIEW!!!
         */
        
        
    public function dispatch()
        {
            
    // an error flag
            
    $error FALSE;
            
            
    // ----------> class finder:
            
            // we have some results here?
            
    if($this->parts){
                
    // Shift an element off the beginning of array (from manual)
                
    $class array_shift($this->parts);
                if (
    $class!=''){
                    
    // sets the server`s path info
                    
    if (count($this->parts)){
                        
    $_SERVER['PATH_INFO'] = '/' implode('/',$this->parts);
                    } else {
                        
    $_SERVER['PATH_INFO'] = '';
                    }
    // end if 
                
    }
                
                
    // TODO: REVIEW!!!
                
    if (file_exists(TOP_LOCATION "components/" $class "/" $class ".php")){
                    include(
    TOP_LOCATION "components/" $class "/" $class ".php");
                }
                else{
                    
    $error TRUE;
                }
                
                if ( 
    class_exists($class)){
                    
    $error FALSE;
                }
                else{
                    
    $error TRUE;
                }   
            }
    // end if array_key_exist
            
            
    else{
                
    $error TRUE;
            }
            
            if(
    $error){
                
    // will give me a default entry point (class) in the application 
                
    $class $this->config->getProperty('entryPoint');
                
                
    $component TOP_LOCATION 'components/' $class "/" $class ".php";
                if(!
    is_file($component)){
                    throw new 
    Exception("ENTRY_POINT definition is wrong! Review the settings",0);
                }
                include_once(
    $component);
            }
            
    // TODO: throw a custom exception
            
    $obj = new ReflectionClass($class);
            if (
    $obj->isInstantiable()) {
                
    $instance $obj->newInstance();
            } else {
                throw new 
    Exception ('Cannot instantiate ' $class ' !',0);
            }
            
            
    // -----------> method finder:
            
            
    $event ''// we will look for this one
            
            
    if(count($this->parts) == 0){
                
    $event 'frontPage'// frontPage is the default method
            
    }
            else {
                
    $event $this->parts[0];
            }

            try{
                
    $method = new ReflectionMethod($class$event);
                if( (
    $method->isPrivate()) OR ($method->isProtected()) OR ($method->isConstructor()) ){
                     
    // event must public 
                    
    $event 'frontPage';
                }
            } catch(
    Exception $ex){
                
    $event 'frontPage';
            }
            
            if(
    $event==''){
                
    $event 'frontPage';
            }
            
    /** PERMISIONS */
            
    $level $this->config->getLevel($class,$event);
            if(
    $level!=0) {
                
    $_SESSION['page'] = $class '/' $event '/';
                
    // echo "Auth req.!<br />";
                
    if(!isset($_SESSION['__auth'])) {
                    
    // echo "1. session auth is not set!";
                    
    $event 'frontPage';
                } elseif( (!isset(
    $_SESSION['__auth']['gid'])) OR ($_SESSION['__auth']['gid'] < $level) ) {
                    
    // echo "2. gr. id is not set or the req. level is higher that the actual level.";
                    
    $event 'frontPage';
                } elseif( !
    $this->auth->compareHashes() ) {
                    
    // echo "3. no such session hash";
                    
    $event 'frontPage';
                }  else {
                     
    // echo "got privileges!";
                
    }
            }
            
            
    /** CALL USER CLASS?METHOD */
            
    $instance->$event();
        } 
    // end dispatch method

    As you can see, I have there a lot of TODO markers.
    I echo things just for testing.
    This is System::getParts():
    PHP Code:
        /**
         * @return array with parts, or FALSE on failure
         */
        
    public final function getParts()
        {
            if (
    array_key_exists('PATH_INFO',$_SERVER)){
                return 
    preg_split('/\//',$_SERVER['PATH_INFO'],-1,PREG_SPLIT_NO_EMPTY);
            }
            else{
                return 
    FALSE;
            }
        } 
    Right now this thing is working, will create a new "component" and will call the method.
    A usual component looks like this:
    PHP Code:
    class news extends Component {

        public function 
    __construct() {
            
    // this will give me a smarty, logger and a DB connection object
           
    parent::_construct(array('smarty','logger','conn'));
        }
         
    // view news 
         
    public function view(){
             echo 
    __METHOD__ "<br />";
         }
         
    // add
         
    public function add(){
             echo 
    __METHOD__ "<br />";
         }
     
    // ...................

    How do I improve this algorithm?
    Specially when trying to find the class Object and the event method.

  2. #2
    eschew sesquipedalians silver trophy sweatje's Avatar
    Join Date
    Jun 2003
    Location
    Iowa, USA
    Posts
    3,749
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    For php4 related code, you might want to check out WACT Handles (code).

    For php5, I have been recently investigating PicoContainter as a framework using Dependency Injection to wire up your application.

    HTH
    Jason Sweat ZCE - jsweat_php@yahoo.com
    Book: PHP Patterns
    Good Stuff: SimpleTest PHPUnit FireFox ADOdb YUI
    Detestable (adjective): software that isn't testable.

  3. #3
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    I start reading your links, including this thread.

    The problem is that I have a system that is working, but adding a new feature (let`s say "component") is painfull.
    Not to say about code duplication.
    Eg. Someone wanted a news "component", if another guy will want the same feature it will be better to rewrite this component rather to use the old one.
    Moreover I plan to move everything to PHP 5.1 (April or May).
    So, I try to refractor the code in small steps.

    The first step: Dispatcher object.

    One major problem is the design of the Dispatcher class. Actually the design of dispatch() method.
    I write the task that are done in dispatch():

    1. CLASS
    1.0 find class,
    1.1 verify the class,
    1.1.1 -> class exists === class is instantiable
    1.1.2 -> file containing this class exists in the system.
    1.2 create an instance of this class

    2. METHOD
    2.1 find method,
    2.2 verify method,
    2.2.1 -> method exists === is a public method
    2.2.2 -> curent user has permision to access this method
    2.3 call this method

    It seams to me that I have a lot of work to do here since there are at least 6 steps on the same method.
    And in my first post you saw that my whole algorithm is a mess.

    I`m not so sure that a Constructor Injection will help me here since I don`t have any arguments passed in the constructors.
    Not even a Setter Injection.
    The idea is that the Dispatcher will not know about "components".
    And the "components" extends from a generic Component witch will build required objects (a form processor will not need a "smarty" object, will do just a redirect to a error/success page) like a database connection, a logger and the smarty template engine object.

    As for pico, I think I need a dummy example
    But I liked the ideea:
    PHP Code:
     $pico->registerComponentImplementation('AmpAdodbConn''AmpAdodbConn',array('db' => new ConstantParameter('prdcrm')); 
    and I will study this post:
    http://www.sitepoint.com/forums/show...8&postcount=14

    Tanx.

  4. #4
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Lightbulb

    Ok, so here is what I have right now:
    PHP Code:
    class Dispatcher
    {
        
    /** URL parts */
        
    private $parts;
        
    /** Config */
        
    private $config;
        
    /** Auth */
        
    private $auth;
        
        
    /**
         * Constructor
         * @access public
         */
        
    public function __construct() {
            
    $this->parts  System::getParts();
            
    $this->config Config::singleton();
            
    $this->auth   = new Auth();
        }
        
        
    /**
         * will instantiate the entry point object with the default event
        */
        
    private function defaultClassCall() {
            
    $class $this->config->getProperty('entryPoint');
            
    $event $this->config->getProperty('defaultMethod');
            
    $file TOP_LOCATION 'components/' $class "/" $class ".php";
            if( (
    is_file($file)) AND (is_readable($file)) ){
                include_once(
    $file);
            } else {
                throw new 
    DispatcherException("ENTRY_POINT definition is wrong! Review the settings",601);
            }
            
    $obj = new ReflectionClass($class);
            if (
    $obj->isInstantiable()) {
                
    $instance $obj->newInstance();
            } else {
                throw new 
    DispatcherException('Cannot instantiate ' $class ' !',602);
            }
            return 
    $instance->$event();
        }
        
        
    /**
         * will call the default method on the given object instance
         */
        
    private function defaultMethodCall($instance) {
            
    $event $this->config->getProperty('defaultMethod');
            return 
    $instance->event();
        }
        
        
    /**  */
        
    public function dispatch() {
            
            
    // ----------> 1. CLASS
            // 1.0 find class,
            
            // we have some results here?
            
    if($this->parts){
                
    // Shift an element off the beginning of array (from manual)
                
    $class array_shift($this->parts);
                if (
    $class!=''){
                    
    // sets the server`s path info
                    
    if (count($this->parts)){
                        
    $_SERVER['PATH_INFO'] = '/' implode('/',$this->parts);
                    } else {
                        
    $_SERVER['PATH_INFO'] = '';
                    }
    // end if class nu e ''
                
    }
            } else {
                
    // failure -> call default class, exit
                
    $this->defaultClassCall();
                return 
    FALSE;
            }
            
    // 1.1 verify the class
            // 1.1.1 -> file containing this class exists in the system.
            
    $file TOP_LOCATION "components/" $class "/" $class ".php";
            if( 
    is_file($file) AND is_readable($file) ) {
                include_once(
    $file);
            } else {
                
    // failure -> call default class, exit
                
    $this->defaultClassCall();
                return 
    FALSE;
            }
            
            
    // 1.1.2 -> class exists === class is instantiable

            
    $obj = new ReflectionClass($class);
            if (
    $obj->isInstantiable()) {
                
    // 1.2 create an instance of this class
                
    $instance $obj->newInstance();
            } else {
                
    // failure -> call default class, exit
                
    $this->defaultClassCall();
                return 
    FALSE;
            }
            
            
    // ----------> 2. METHOD
            // 2.1 find method,
            
            
    if(count($this->parts) == 0) {
                
    // failure -> default event, must exit
                
    $this->defaultMethodCall($instance);
                return 
    FALSE;
            }
            else {
                
    $event $this->parts[0];
            }
            
            
    // 2.2 verify method,
            // 2.2.1 -> method exists === is a public method

            
    try{
                
    $method = new ReflectionMethod($class$event);
                if( (
    $method->isPrivate()) OR ($method->isProtected()) OR ($method->isConstructor()) OR ($event == '') ) {
                    
    // failure -> call default event, must exit
                    
    $this->defaultMethodCall($instance);
                    return 
    FALSE;
                }
            } catch(
    Exception $ex){
                
    $this->defaultMethodCall($instance);
                return 
    FALSE;
            }
            
            
    // 2.2.2 -> curent user has permision to access this method
            
    $reqLevel $this->config->getLevel($class,$event);
            
    // authentication is req. for this method?
            
    if($reqLevel != 0) {
                
    // remember last page:
                
    $_SESSION['page'] = $class '/' $event '/';
                
    // generic message:
                
    $message "Authentication required!<br />";
                
    // validation:
                
    if(!isset($_SESSION['__auth'])) {
                    
    // sess auth is not set.
                    
    $instance->redirect('/admin/login'$message 'Session authentication is not set!',5);
                    
                } elseif( (!isset(
    $_SESSION['__auth']['gid'])) OR ($_SESSION['__auth']['gid'] < $level) ) {
                    
    // gr. id is not set or the req. level is higher that the actual user level
                    
    $instance->redirect('/admin/login',$message 'Group id is not set or 
                                        the required level is higher that the actual level'
    ,5);
                
                } elseif( !
    $this->auth->compareHashes() ) {
                    
    // session hash is broken
                    
    $instance->redirect('/admin/login',$message 'Failed to check your session hash!',5);
                }  else {
                     
    // nothing more to do here
                
    }
            }
            
    // 2.3 call this method
            
    return $instance->$event();
        }

    I have more code here, but this time is human readable.
    The main component:
    PHP Code:
    abstract class Component {

    // atributes, object containers
    // like protected $smarty=null, $conn=null,$logger=null;
    // they will be build in constructor on special request from child classes

    public function __construct($params=array()) {
    // instantiate objects passed in the array
    }
    // this way I will not forgot about default method
    abstract protected function frontPage();

    public function 
    redirect($location,$message,$timeout){
    // will redirect the user to $location...
    }

    Some comments?
    Hints?

  5. #5
    eschew sesquipedalians silver trophy sweatje's Avatar
    Join Date
    Jun 2003
    Location
    Iowa, USA
    Posts
    3,749
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I have a pretty simple idea of how a web application can work that seems to get me by for the most part. It is basically the double switch dispatch I refered to recently in the Procedureal Design Patterns thread.

    The first opportunity to dispatch is for "Actions". I uses these when an application needs to change state, i.e. form processing, etc. My actions are essentially the Strategy pattern, as the all respond to a "process()" interface. My code to dispatch to an action determines if an action is requested, looks up in a mapping what class should responds to the requested action, make sure that class in included, instantiates the action and calls the process() method. All of my actions redirect to either another action or to a view.

    The second dispatching is for "Views". I currently use a WACT Parameter controller to handle this dispatching, but essentially it is similar to the actions: figure out which one, make sure it is included, instantiate, load it up (common prepare() method) and then display it (also a common method).

    If I understand what you are doing, you are adding several layers of complexity on that. You have the possibility of requesting objects which can't be instantiated? You also are allowing for variable methods to be called on these objects.

    The WACT way of dealing with similar complexities is the EventListeners, but I am afraid I have not devled into them to much, as I have stayed with the simpler double dispatch design.

    Sorry to not be of more help.

  6. #6
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    I think the idea is simple here.
    In a index.php file I have smthing like this:
    PHP Code:
    include_once('config.php'); // only for setting some common php stuff (error_reporting(E_ALL|E_STRICT) and so on)
    include_once('libs/Dispatcher.php');
    include_once(
    'libs/System.php');
    $req = new Dispatcher();
    try{
        
    $req->dispatch();
    } catch(
    Exception $e) {
        echo 
    System::formatMessage($e);

    In my prev. post I put Dispatcher.php and Component.php

    Incoming URL looks like this:
    http://example.com/index.php/news/details/1
    where news is the object to be called, details is the method from news and 1 is the news id (I will grab this one from DB).
    Dispatcher will instantiate the news object and will call the details method.

    In the news constructor I instantiate other objects: smarty template engine, a DB connection (or handler), a logger to log errors.
    In details method I get all the infos for news id 1 and assign them to smarty.

    I have read a lot of posts from this forum and code (prado, WACT, seagull etc. etc.) but I`m not so sure (fear?) how should I start improving what I already have.

    But I`m aware that the system right now has a lot of "holes".
    uff

  7. #7
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hello,

    Not got the time to read your posts SweatJe at the moment, but...

    It seams to me that I have a lot of work to do here since there are at least 6 steps on the same method.
    In this case, I'd seperate this out of the Dispatcher class? The Dispatcher class I assume has a very specific task to do, so surely the verification and retrieval of the class required can be delegated to another class?

    Wouldn't this give a cleaner Dispatcher interface, and allow easier refactoring I thinking

  8. #8
    SitePoint Enthusiast
    Join Date
    Sep 2004
    Location
    Bucharest
    Posts
    30
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    What is the "special" task of a dispatcher?
    It`s the same thing as a Front Controller?

    Some reading:
    http://www.phppatterns.com/index.php...leview/81/1/1/

  9. #9
    SitePoint Wizard
    Join Date
    Aug 2004
    Location
    California
    Posts
    1,672
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I have still not found the best mix of classes for a PHP controller architecture. I think I'm on my 3rd or 4th generation. It has gotten to the point where one of my design goals is to be able to emulate the old controller system in the new one to enable new features, then migrate by refactoring.

    In looking at your code, I tend to split your Dispatcher class into three parts. Referring to your original post of code, for the first third of your code I have a separate ActionMapper class that deals with turning external requests into internal path/class/method information.

    The second third of the code there is the actual controller part, and yes it does need to do all the checks to see if the class and method exists.

    The final third of your Dispatcher class is access control. I usually implement as an installable component. I think an Intercepting Filter type system, either in the front or secondary controller, works for this. Hewever I have been leaning more and more toward integrating it into the controller.

    I have never been able to get a single Intercepting Filter controller design to work the way I wanted it to, so do the two-step like sweatje mentioned. I actually think that this Double Switch Controller pattern is a new pattern that has evolved within PHP to deal with the simple request model that PHP uses.
    Christopher

  10. #10
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    where news is the object to be called, details is the method from news and 1 is the news id (I will grab this one from DB).
    What frighten's me is that you are passing a variable via the GET/POST that is later used to determine a class methods execution. Forgetting for a moment the security aspects of this, why do you need to?

    To me, it looks like you'll have a class method in a given class for every action to perform, or I should say for every component you have, if I'm following this...

    Dispatcher will instantiate the news object and will call the details method.
    No problem passing the component to use via GET/POST (ie News) but why don't or haven't you used a generic class method... One that each component would inherit...

    Could you explain some more behind your reasoning please as I don't follow the benifits of what your script offers

  11. #11
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Question

    Quote Originally Posted by Dr Livingston
    What frighten's me is that you are passing a variable via the GET/POST that is later used to determine a class methods execution.
    What is the diffrence between the method that I`m using right now and something like:
    http://www.example.com/index.php?page=news&news_id=1 ?
    and in the index.php:
    PHP Code:
    <?
    include(header.php);
    include(
    $_GET['page']);
    include(
    footer.php);
    ?>
    I think is a major difference since in the Dispatcher I do all the security checks.
    I make sure that the class exist, the method is there and the user has permision to access that resource.

    The generic method is Component, parts of this abstract object are posted in a prev. post. Every component (like news) extends from the generic component.
    It`s an abstract class because I put there an abstract method (frontPage) witch must be overwriten by children. This way I will not forget to add the default method (frontPage, I think it was main() in the article->link in my first post).

    Moreover, the code for generic Component (I post again):
    PHP Code:
    <?php
    abstract class Component{
        protected 
    $smarty null;
        protected 
    $conn null;
        protected 
    $logger null;
        
    /**
         * is the constructor
         * will build requested objects
         */
        
    protected function __construct($params=array()){

            if(!empty(
    $params)){
                foreach(
    $params as $key=>$value){
                    if(
    $params[$key] == 'logger'){
                        
    // instantiate the logging system:
                        
    $this->logger = new Logger();
                    } elseif(
    $params[$key]=='smarty'){
                        
    // start smarty template engine:
                        
    $this->smarty = new Smarty();
                    } elseif(
    $params['key'] == 'conn'){
                        
    // a DB connection
                        
    $this->conn = new DBConnection($dsn);
                    } else {
                        throw new 
    Exception('No such object defined!');
                    }
                } 
    // end foreach
            
    // end if
        
    // end constructor
        /**
         * will do a redirect
         */
        
    public function redirect($location,$message,$timeout=1) {
            
    $this->smarty->assign('location',$location);
            
    $this->smarty->assign('message',$message);
            
    $this->smarty->assign('timeout',$timeout);
            
    $this->smarty->display('redirect.tpl');
        }

        
    /**
         * It displays common things on the page
         */
        
    protected function common($params=array(),$title='',$display='index.tpl') {
            
            foreach(
    $params as $key=>$value){
                
    $this->smarty->assign($key,$value);
            }
            
    // title is the page title, the content of <title>{$title}</title> in template:
            
    $this->smarty->assign('title',$title);
            
    $this->smarty->display($display);
        }
        
        abstract public function 
    frontPage();
        
    }
    Other methods are still provided here (Eg. isPageCompiled() and so on).
    And for news object I will have:
    PHP Code:
    <?php

    class news extends Component{
        public function 
    __construct() {
            
    parent::__construct(array('logger','smarty','conn'));
        }

        
    /**
         * the news front page
         * usualy here we want to see the last 5 headlines
         */
        
    public function frontPage() {
            
    $lastNews $this->getLastNews();
            
    $params = array('lastNews'=>$lastNews);
            
    $this->common($params,'Last News','lastnews.tpl');
        }

        
    /**
         * will show the details about a news
         */
        
    public function details() {
            
    $newsId = (int)__GET__THE__NEWS__ID__FROM__URL__;
            
    $newsDetails $this->getNewsDetails($newsId);
            
    $params = array('newsDetails'=>$newsDetails);
            
    $this->common($params,'Details About ' $newsId,'details.tpl');
        }

        
    /**
         * will add a new news
          */
        
    public function add(){
            
    // just show a form
        
    }
        
    /**
         * process the form
          */
        
    public function process(){
            
    // do usual stuff like validation, insert in DB
        
    }

        
    /**
         * will give the last 5 news
         */
        
    private function getLastNews() {
            return 
    _SQL_QUERY_RESULT_SET_TO_GET_THE_LAST_5_NEWS_;
        }
        
    /**
         * will give me the news details
         */
        
    private function getLastNews() {
            return 
    _SQL_QUERY_RESULT_SET_TO_GET_THE_NEWS_DETAILS_;
        }

        
    /**
         * for tracking the user?
         */
        
    public function __destruct() { }
    In Dispatcher I`m making sure that only public methods are called and for example the add and process methods are called by an authenticated user.

    This system is still very flexible but I realise that the Dispatcher is changing eveery time. On the second place is the DB abstraction layer (was pure mysql_query, ADODB, PEAR: : DB and now Creole, it will be PDO asap).
    Other 3rd party libs are easily integrated in this system (like: phpOpenTraker, DB_Nested_Set etc.).

    Dr. how I will improve the design of this system? Not only the design of my actual Dispatcher.
    Who is responsable for all the dirty work in an application?
    Dirty Work: find the class, find the method, apply filters.
    Where should I put this algorithm?

    Another "component", a gallery system ended up with almost 700-800 lines of code. Is too much I think for one class. Not to say is very hard to add a new feature there.

    arborint, as you can see my "controller" is almost in the same situation regarding the generation of this library.
    And thanks for your advices, I will probably split my Dispatcher in 3 parts:
    -> find the class
    -> find method
    -> apply filters (authentication etc.)

    fastwork: Thanks for the link.
    I`m not planning to make this a 15 pages thread where we all argue about a name and we end up with nothing.

    So, I will wait for your comments, and how do I improve not only the Dispatcher design, but also the whole system design.
    (design AKA design of the code).
    And all in small steps

  12. #12
    eschew sesquipedalians silver trophy sweatje's Avatar
    Join Date
    Jun 2003
    Location
    Iowa, USA
    Posts
    3,749
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by topsmith
    PHP Code:
    <?
    include(header.php);
    include(
    $_GET['page']);
    include(
    footer.php);
    ?>
    Aaahhhh!!!! No, No, No!!!!

    Think about include($_GET['page']);
    with the url http://www.example.com/path/to/index...2Fetc%2Fpasswd

    where %2Fetc%2Fpasswd unurlencode is /etc/passed!!!

  13. #13
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    No way, I will not do anything like that

    I sugested only to make a comparison between that style and what I have right now.
    Another thread:
    http://www.sitepoint.com/forums/show...2&postcount=16
    or more recent:
    http://www.sitepoint.com/forums/showthread.php?t=239167
    PHP Code:
    switch ($_GET['page'])
            {
                case 
    'add':
                    
    $command = new UserAddCommand;
                    break;

                case 
    'edit':
                    
    $command = new UserEditCommand;
                    break;

                case 
    'list':
                    
    $command = new UserListCommand;
                    break;
            } 
    I think we all have some design problems.

  14. #14
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I`ve changed the "component", this time a component is a folder where I keep all related objects with (let`s say) news.
    So, I have components (news,gallery) and actions: AddNews, ViewAllNews, EditNews.
    All my actions will extend from a base Action (it was Component) and they will have a default method execute() (as someone sugested here).

    TOP_LOCATION . "components/news/" is the location of news.
    AddNews.php
    EditNews.php
    AddCategory.php
    ..........................
    TOP_LOCATION . "components/gallery/" is the location of a gallery.
    AddCategory.php
    EditPicture.php
    AddPicture.php
    PictureDetails.php
    and so on.

    I`ve changed the old System : : getParts():
    PHP Code:
    <?php
    class Request{
        
        
    /** request ->former parts */
        
    private $request;
        
        
    /** constructor, will set the request */
        
    public function __construct() {
            if (
    array_key_exists('PATH_INFO',$_SERVER)){
                
    $this->request preg_split('/\//',$_SERVER['PATH_INFO'],-1,PREG_SPLIT_NO_EMPTY);
            }
            else{
                
    $this->request null;
            }
        }
        
        
    /** will get the request */
        
    public function getRequest() {
            return 
    $this->request;
        }
    }
    The major change in my design is the RequestProcessor, witch will replace my old Dispatcher. (based on struts/RequestProcessor).
    Here I will do all the processing logic, including: selecting the action, create an instance of this action and call the execute method.

    3 questions:
    What do you think about fail-fast concept? (I kinda like to fail-fast in my code)
    What do you think about returning null values? Should I replace them with boolean: false?
    Did I made some progress?
    Last edited by topsmith; Mar 3, 2005 at 07:18. Reason: typo :)

  15. #15
    Non-Member
    Join Date
    Jan 2003
    Posts
    5,748
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    What do you think about returning null values? Should I replace them with boolean: false?
    I would, so you could do this easily for example,

    PHP Code:
    ...
    if( !
    $expected $this -> callSomeMethod$parameter ) ) {
    // do something positive
    } else { // kill the messenger?
    }
    ... 
    I never use NULL so I'd expected it'd be slightly more work to determine a result?

  16. #16
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    -> renamed back Request to Parts since in this thread:
    http://www.sitepoint.com/forums/showthread.php?t=225335
    I found some good points.
    -> using false not null anymore (tanx Dr Livingston).

    karma?

  17. #17
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Post

    Ok, so here is what I have right now:
    Parts.php
    PHP Code:
    <?php
    class Parts {
        private 
    $parts;
        
        
    /** constructor, will set our parts */
        
    public function __construct() {
            if (
    array_key_exists('PATH_INFO',$_SERVER)){
                
    $this->parts preg_split('/\//',$_SERVER['PATH_INFO'],-1,PREG_SPLIT_NO_EMPTY);
            }
            else{
                
    $this->parts false;
            }
        }
        
        
    /** will get the parts */
        
    public function getParts() {
            return 
    $this->parts;
        }
    }
    I replace the Dispatcher with a RequestProcessor (as I saw in struts framework). It looks smthing like this:
    PHP Code:
    class RequestProcessor{
        
        
    /** starts a logger engine */
        
    public function __construct(){ }

        
    /** I put Parts here as an argument just for fun
              and to see how Type Hinting is working on php */
        
    public function process(Parts $parts) {
            
    $this->parts=$parts->getParts();
            
    /** will check the
                  URI, include req. files instantiate the action class
                  or call a default component/action if there is a failure
            */
            
    $component  $this->processComponent();
            
    $class        $this->processClass(); // or view, it will be better.
           /** other filters: */
           
    $this->processLocale();
           
    /** authentication */
           
    $role $this->processRole($component,$class);
           
    /** a check */
           
    if(!$role) {
               
    // redirect user to a login form,
               // remember the component and action is a session variabile
               // after the login, the user will be redirected to the old requested page
          
    }
          
    /** a actionView instance: */
          
    $action $this->processViewCreate($class);
          
    /** create a model instance for this action */
          
    $model $this->processActionModelCreate($class);
          
    /** run the action: */
          
    $action->execute($model);
    }

    There is no magic in my filters (process* methods) .
    Also, I`ve made two abstact classes: ActionView and ActionModel.
    In ActionView I create a render engine instance (in this case is smarty), and I call the execute() method from ActionModel witch will return all the informations needed to display the page.
    In ActionModel, a DB Connection Insance is created, some sql querys (or other methods to grab the informations) and will return a ready to go array (it will be passed to smarty).

    I choose to keep the ideea with components because this way my code will be organized.

    Again, a component is a folder(!) where I keep all the related objects with it.

    Eg.: for a gallery system, the folder gallery is the component, on witch resides actions related with gallery:

    addpictureView.php <-- --> addpictureMoldel.php,
    editpictureView.php <-- --> editpictureModel.php,
    allpicturesView.php <-- --> addpictureModel.php and so on.

    Same for a news engine or whatever.

    Permision system is based on user/groups stored in sql DB and a xml permision file witch will map the components/actions to group level.
    This way, at some points I can build a menu for a admin, a regular user menu and so on.

    General configuration is also made in a an xml file. (DB login infos, Session related, loggin system->enable/disable debug for example, application state, render engine options -> use/not use cache, template folder).

    Questions or Remarks?

  18. #18
    SitePoint Evangelist
    Join Date
    Jun 2003
    Location
    Melbourne, Australia
    Posts
    440
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by topsmith
    I`m not planning to make this a 15 pages thread where we all argue about a name and we end up with nothing.
    Gee, topsmith, you've been here such a short time and you've already figured out how we do things!
    Zealotry is contingent upon 100 posts and addiction 200?

  19. #19
    SitePoint Enthusiast topsmith's Avatar
    Join Date
    Feb 2005
    Posts
    35
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Hi.

    Now I`m happy with the design, I think I did a good job, and since I don`t have any other questions on this topic, "Improvinig Dispatcher Design" saga has come to an end.

    Regards, topsmith.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •