Jump to content

[SOLVED] Design question


Stooney

Recommended Posts

I'm half confused on this.  I have a registry as a singleton which for the most part stores my objects to be used wherever they're needed.  For example a logging class which logs whatever it is I need to log.  Now how should it be accessed?

 

Say my Database class has problems with a query and I want to log it so I can go back and fix it.  Should I just have an instance of the logging class in my database class?

 

<?php
class database{
    private $log;

    __construct($host, $user, $pass, $dbname, $log){
        //connect and stuff
        $this->log=$log;
    }

    public function query($query){
        //query fails, log it
        $this->log->log('sql', 'Failed Query: '.$query);
    }
}

 

This is how registry works

<?php
$registry=Registry::getInstance();
$log=new Log(...);
$registry->set('log', $log);

//etc for other objects
?>

 

That was just an example.  If I did it that way then there would be multiple instances of each class in multiple objects which doesn't seem like the right way to do it.  So what's the better way of doing this?  It seems like a common thing, I just can't figure out the design.

 

(This is all example code, so excuse typos etc.  Edit:  A quick read through the sticky makes me think this should have been in app design, whoops)

Link to comment
Share on other sites

Well, since a database layer doesn't seem to inherently have anything to do with logging, you could always throw exceptions:

 

try {
    $db->query('something');
} catch (DbQueryException $e) {
    $log->log($e, Log::EXCEP);
}

 

And $db->query() could look like:

    public function query($query)
    {
        //do query
        if (query_failed) {
            throw new DbQueryException("Query $query failed");
        }
    }

Link to comment
Share on other sites

That makes sense DarkWater, thank you.  What about with an MVC architecture?  Should I just have a private $log variable for each controller?  Maybe just pass the whole registry to the controller?

 

As it stands, I instantiate my classes (db, log, auth, etc) at the start of the site.  They're then stored in the registry object (which is a singleton).  After all this the router loads the proper controller and invokes the requested method.  Naturally that method (or entire controller for that matter) doesn't have access to the registry.  Should I pass the entire registry object to my controller?  Or just the necessary parts depending on the controller (which might be tricky considering the router object doesn't know anything about that controller). 

Link to comment
Share on other sites

You have several choices.  Since it's a singleton, you could always just statically get it and use it globally:

 

class Foo {
    public function index() {
        $log = Registry::get('log');
        //do stuff, use $log, etc
    }
}

 

Or, you could have a base class that all controllers inherit from and set it up like this:

 

<?php
class BaseController {
private $set_vars = array();

public function __construct() {
	//potentially do something here, and if a controller wants to override, have them call parent::__construct()
}

public final function __set($key, $value) {
	$this->set_vars[$key] = $value;
}

public final function __get($key) {
	if (array_key_exists($key, $this->set_vars)) {
		return $this->set_vars[$key];
	}
	throw new Exception('Illegal key retrieval');
}
}

 

Then, when you prepare to call the controller method, do something like:

if (class_exists($controller)) {
$control = new $controller();
}
else {
throw new Exception('Bad controller name');
}

if (method_exists($action, $control)) {
                $control->log = new Log();
                $control->view = new BaseView();
                call_user_func_array(array(&$control, $action), $params);
}
else {
        throw new Exception('Bad action name');
}

 

Now, I just conjured that up now, so it might have some types.  I know that the BaseController class doesn't though, because I tested it.

Link to comment
Share on other sites

Thank you very much DarkWater.  It all makes sense now.  I think I prefer the static method.

 

Yeah, I figured you'd say that.  I just want to let you know that it's not REALLY the best way to do it, I was just showing you the option.  Having a base class and properly instantiating the controller and giving it $this->log and stuff is (imo) a much better plan.

Link to comment
Share on other sites

This is what my router looks like right now.  I just noticed I could do away with setPath and do that in the construct, I'll fix that later.  You'll also notice some comments where code should be, I'm working on write that stuff still (like //show 404)

 

<?php
class Router{
private $path;

public function __construct(){
	//Nothing
}

public function setPath($path){
	$path=trim($path, '/\\');
	$path.=DS;

	if(is_dir($path)){
		$this->path=$path;
		return true;
	}
	throw new Exception('Invalid controller path');
}

private function getController(&$file, &$controller, &$action, &$args){
	if(isset($_GET['d']) && !empty($_GET['d'])){
		$destination=$_GET['d'];
	} else{
		$destination='index';
	}

	//Get Separate Parts
	$destination=trim($destination, '/\\');
	$parts=explode('/', $destination);

	//Find Controller
	foreach($parts as $part){
		$fullpath=$cpath.'Controller_'.$part;

		//Check if $fullpath is a dir
		if(is_dir($fullpath)){
			$cpath.=$part.DS;
			array_shift($parts);
			continue;
		}

		//Find the file
		if(is_file($fullpath.'.php')){
			$controller='Controller_'.$part;
			array_shift($parts);
			break;
		}
	}
	if(empty($controller)){
		$controller='Controller_index';
	}

	//Get Action
	$action=array_shift($parts);
	if(empty($action)){
		$action='index';
	}
	$file=$cpath.$controller.'.php';
	$args=$parts;
}

public function delegate(){
	//Analyze destination
	$this->getController($file, $controller, $action, $args);

	//File available?
	if(!is_readable($file)){
		//Show 404
		return false;
	}

	//Include the file
	include($file);

	//Instantiate the class
	$class=$controller;
	$controller=new $class();

	//Action available?
	if(!is_callable(array($controller, $action))){
		//Show 404
		return false;
	}

	//Run action
	$controller->$action($args);
}
}
?>

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.