Stooney Posted October 27, 2008 Share Posted October 27, 2008 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) Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/ Share on other sites More sharing options...
DarkWater Posted October 27, 2008 Share Posted October 27, 2008 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"); } } Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676046 Share on other sites More sharing options...
Stooney Posted October 27, 2008 Author Share Posted October 27, 2008 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). Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676116 Share on other sites More sharing options...
DarkWater Posted October 27, 2008 Share Posted October 27, 2008 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. Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676135 Share on other sites More sharing options...
Stooney Posted October 27, 2008 Author Share Posted October 27, 2008 Thank you very much DarkWater. It all makes sense now. I think I prefer the static method. Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676145 Share on other sites More sharing options...
DarkWater Posted October 27, 2008 Share Posted October 27, 2008 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. Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676149 Share on other sites More sharing options...
Stooney Posted October 27, 2008 Author Share Posted October 27, 2008 If it's the better option than that's what I will do. I'd rather learn than cut corners. Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676175 Share on other sites More sharing options...
DarkWater Posted October 28, 2008 Share Posted October 28, 2008 Can I see how you currently route requests and call your controllers and stuff? Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676182 Share on other sites More sharing options...
Stooney Posted October 28, 2008 Author Share Posted October 28, 2008 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); } } ?> Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676209 Share on other sites More sharing options...
DarkWater Posted October 28, 2008 Share Posted October 28, 2008 Try adapting my routing code to your class, and you can also use call_user_func_array() to pass in an array of parameters instead of just one flat array. Make sure to inherit all controllers from BaseController. =P Quote Link to comment https://forums.phpfreaks.com/topic/130232-solved-design-question/#findComment-676225 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.