Page 1 of 2

many pages, or one page with $_GET parameter

Posted: Sat Aug 11, 2007 7:37 pm
by s.dot
Say you're making a survey script (which I am, btw).

Do you make the pages..

survey_main.php
survey_take.php
survey_create.php
survey_browse.php

Or, do you make it one script with an if/else/elseif? surveys.php?cmd=take, surveys.php?cmd=create.. etc?

Usually I make separate scripts, but lately I've been combining them into one. Which isn't bad because all of my "pages" are in one file, and I can quickly and easily access the whole "category" of pages. The readability isn't too bad either, alongside some whitespace and comments. But, I know separation of tasks is good also. What are your thoughts on this?

Posted: Sat Aug 11, 2007 7:41 pm
by superdezign
It depends on how closely related they are. If they perform totally different tasks, then they shouldn't be grouped together. Yours seem like they should be separate files in one folder, to me.

Posted: Sat Aug 11, 2007 8:03 pm
by Chris Corbyn
I see where you're heading scottayy with your recent posts so I'll post some food-for-thought ;)

Code: Select all

<?php

class FrontController {
  protected static $instance = null;
  public static function getInstance() {
    if (self::$instance === null) {
      self::$instance = new self();
    }
    return self::$instance;
  }
  public function dispatch() {
    $page = !empty($_GET["page"]) ? $_GET["page"] : "home";
    $action = !empty($_GET["action"]) ? $_GET["action"] : "index";
    $class = ucfirst($page) . "Controller";
    $file = PAGE_DIR . "/" . $class . ".php";
    if (!file_exists($file)) {
      exit("Page not found");
    }
    require_once $file;
    $page = new $class();
    $page->dispatchAction($action);
  }
}
index.php:

Code: Select all

<?php

define("PAGE_DIR", dirname(__FILE__) . "/pages");
require_once "FrontController.php";
FrontController::getInstance()->dispatch();
ActionController.php:

Code: Select all

<?php

abstract class ActionController {
  public function dispatchAction($action) {
    $actionMethod = "do" . ucfirst($action);
    if (!method_exists($this, $actionMethod)) {
      die("Action not found");
    }
    $this->$actionMethod();
  }
}
pages/SurveyController.php:

Code: Select all

<?php

require_once dirname(__FILE__) . "/../ActionController.php";

class SurveyController extends ActionController {
  public function doIndex() {
    echo "Index called...";
  }
  public function doCreate() {
    echo "Create called...";
  }
}
Now point your browser to:

http://localhost/index.php?page=survey&action=create

Should get you started with a little home-brewed framework :) Now you just need to work out how to tie in a View system ;)

Posted: Sat Aug 11, 2007 10:12 pm
by Christopher
Here is my spin on d11's excellent code. It eliminates the ActionController class (until you have a need for one) and filters the input a little. I also don't see how you could or would ever create multiple Front Controllers as it is only ever invoked in index.php -- so I removed the Singleton support. This also of follows Zend Framework naming conventions.

Code: Select all

<?php

class FrontController {
  protected $controller_dir;
  protected $default_controller;
  protected $default_action;
  protected $error = 0;

  public function __construct($controller_dir, $default_controller, $default_action) {
    $this->controller_dir = $controller_dir;
    $this->default_controller = $default_controller;
    $this->default_action = $default_action;
  }

  public function dispatch() {
    $dispatch['controller'] = isset($_GET['controller']) ? preg_replace('/[^a-zA-Z0-9\_]/', '', $_GET['controller']) : '';
    $dispatch['action'] = isset($_GET['action']) ? preg_replace('/[^a-zA-Z0-9\_]/', '', $_GET['action']) : '';
	if (! $dispatch['controller']) {
    	$dispatch['controller'] = $this->default_controller;
	}
    if (! $dispatch['action']) {
        $dispatch['action'] = $this->default_action;
    }
    do {
            $this->error = 0;
 	    $class = ucfirst($dispatch['controller']) . "Controller";
	    $method = $dispatch['action'] . 'Action';
	    $dispatch = array();	// clear for forward
	    
	    $file = $this->controller_dir . "/" . $class . ".php";
	    if (file_exists($file)) {
	       require_once $file;
	       if (class_exists($class)) {
		       $obj = new $class();
		       if (method_exists($obj, $method)) {
		          $dispatch = $obj->$method();		// execute and get optional forward
		       } else {
		         $this->error = 3;
		       }
	       } else {
	         $this->error = 2;
	       }
	    } else {
	      $this->error = 1;
	    }
    } while ($dispatch);
    return $this->error == 0;
  }
  
  public function getError() {
  	return $this->error;
  }  
}
index.php:

Code: Select all

<?php

require_once "FrontController.php";
$fc = new FrontController('controller', 'home', 'index');
if (! $fc->dispatch()) {
    // error occured
}
controller/SurveyController.php:

Code: Select all

<?php

class SurveyController {
  public function indexAction() {
    echo "Index called...";
  }
  public function createAction() {
    echo "Create called...";
    return array('controller'=>'index', 'action'=>'foo');      // forward by returning controller/action
  }
}
http://localhost/index.php?controller=s ... ion=create

Posted: Sat Aug 11, 2007 11:45 pm
by Benjamin
If your making what I think your making, a simple switch would do the job.

Posted: Sun Aug 12, 2007 1:08 am
by Christopher
astions wrote:If your making what I think your making, a simple switch would do the job.
If you are thinking what I think you are thinking, you are not understanding the Front Controller pattern.

Posted: Sun Aug 12, 2007 1:13 am
by Benjamin
arborint wrote:
astions wrote:If your making what I think your making, a simple switch would do the job.
If you are thinking what I think you are thinking, you are not understanding the Front Controller pattern.
That wasn't a very nice comment. I reread the original post and he is not making what I thought he was making.

Posted: Sun Aug 12, 2007 2:47 am
by Christopher
astions wrote:That wasn't a very nice comment. I reread the original post and he is not making what I thought he was making.
Not intended to be not nice. A switch can certainly be used for a Front or Page controller, but it has the limitations of not handling large numbers of controllers that well and that you have to change the dispatch code every time you add, remove or rename an action.

Posted: Sun Aug 12, 2007 3:05 am
by Benjamin
I thought he was making a facebook application.

Posted: Sun Aug 12, 2007 5:46 am
by Chris Corbyn
:lol:

I did post the ActionController class for a reason, although I should really have had a "Controller" class which but FrontController and ActionController extend. I didn't want to overwhelm though ;) See, now that you've got all responsibility for invoking actions in the front controller it's a little more difficult to control flow between controllers. I guess you have your own clean way of doing that too, but I was going to post this next... I'd be interested in starting a new thread and discussing advtantages and disadvantages in depth because I know you have some great ideas on this sort of stuff. I find it an interesting topic too.

I'm throwing in forwarding here with only a little modification:

Code: Select all

<?php

abstract class Controller {
  //as in $this->getRequest()->getParameter() and $this->getRequest()->getSession()
  public function getRequest() {
  }
  //as in $this->getResponse()->setHeader() and $this->getResponse()->sendRedirect()
  public function getResponse() {
  }
  public function forward($page, $action) {
    $class = ucfirst($page) . "Controller"; 
    $file = PAGE_DIR . "/" . $class . ".php"; 
    if (!file_exists($file)) { 
      exit("Page not found"); 
    } 
    require_once $file; 
    $page = new $class(); 
    $page->dispatchAction($action);
    exit(0); //Ok
  }
}

Code: Select all

<?php

require_once dirname(__FILE__) . "/Controller.php";

class FrontController extends Controller { 
  protected static $instance = null; 
  public static function getInstance() { 
    if (self::$instance === null) { 
      self::$instance = new self(); 
    } 
    return self::$instance; 
  } 
  public function dispatch() { 
    $page = !empty($_GET["page"]) ? $_GET["page"] : "home"; 
    $action = !empty($_GET["action"]) ? $_GET["action"] : "index"; 
    $this->forward($page, $action);
  }
}

Code: Select all

<?php 

abstract class ActionController extends Controller {
  public function dispatchAction($action) { 
    $actionMethod = "do" . ucfirst($action); 
    if (!method_exists($this, $actionMethod)) { 
      die("Action not found"); 
    } 
    $this->$actionMethod();
  } 
}
pages/SurveyController.php

Code: Select all

<?php 

require_once dirname(__FILE__) . "/../ActionController.php"; 

class SurveyController extends ActionController { 
  public function doIndex() { 
    echo "Index acton..."; 
  } 
  public function doCreate() {
    $this->forward("survey", "something"); 
    echo "Create action..."; //Never runs
  }
  public function doSomething() {
    echo "Something action...";
  }
}
View inclusion would happen in the ActionController's dispatchAction() method only if the controller doesn't explicitly disable the view whilst running the action.

Posted: Sun Aug 12, 2007 6:53 am
by s.dot
astions wrote:I thought he was making a facebook application.
Close! I am making it for my myspace "help" site. As if there aren't enough, eh? =] The coding is absolutely no problem at all. Procedurally, I feel like I can code *almost* anything I want to, so I want to move up a level while I'm doing something for fun that doesn't really matter.

The code aborint and d11wtq posted is way above my head. But I'm already trying to break it down into pieces. I'm sure I'll get it soon enough (.. or later enough).

Posted: Sun Aug 12, 2007 12:29 pm
by superdezign
@arborint: Maybe I'm reading your code wrong, but I think you mixed up the true and false in dispatch().

BTW, these code examples are extremely helpful. My controllers were all hard-coded into pages because I couldn't think of a way to organize them, but these examples have made it much clearer. Kudos to the both of you. :-D
Luckily for me, I was already re-writing my code. ^_^

Posted: Sun Aug 12, 2007 1:27 pm
by Chris Corbyn
superdezign wrote:@arborint: Maybe I'm reading your code wrong, but I think you mixed up the true and false in dispatch().

BTW, these code examples are extremely helpful. My controllers were all hard-coded into pages because I couldn't think of a way to organize them, but these examples have made it much clearer. Kudos to the both of you. :-D
Luckily for me, I was already re-writing my code. ^_^
~superdezign, I took the liberty of writing a short blog entry extending upon the sort of code I posted here ;) If it's of any help:

http://www.w3style.co.uk/a-lightweight- ... -for-php-5

This one does include the view, albeit not a composite one, but it's not hard to modify to work with a composite view.

Posted: Sun Aug 12, 2007 1:29 pm
by Christopher
superdezign wrote:@arborint: Maybe I'm reading your code wrong, but I think you mixed up the true and false in dispatch().
You are absolutely right. I sort of fixed it.

Posted: Sun Aug 12, 2007 2:10 pm
by superdezign
d11wtq wrote:This one does include the view, albeit not a composite one, but it's not hard to modify to work with a composite view.
Then I'll certainly give it a read. ^_^
I just got to the view and was trying to decipher a good method for using the consistent outer template with the inner templates.