application design?

[thorpe]

ok... so im not sure if anyone will be able to see whats going on, or even if you can critique such an application without seeing the underlying classes, but i just wanted to get some opinions on the overall logic of my cms application.

this is the first thing ive made in php and its all working well, just wanted to get some ideas.

ive built all the classes myself, and there is also an underlying mysql class that is called upon by most of the classes.

i think my template class might be holding me back a little, but i allready have some ideas on how to improve it.

anyway, if you can see the logic in this code and have any thoughts on the design... would be great.

<?php

require_once('core/includes/application.config.php'); // must be called first.
require_once('core/includes/functions.php'); // must be called second.
require_once('core/classes/class.permisions.php');
require_once('core/classes/class.user.php');
require_once('core/classes/class.template.php');
require_once('core/classes/class.menu.php');
require_once('core/classes/class.threads.php');
require_once('core/classes/class.articles.php');

if (!session_id()) { // check user is logged in
session_start(); // setup a default session
$_SESSION['upermisions'] = DEFAULT_USER_PERMISIONS;
}

$threads = new threads(); // create thread object
$tid = $threads->check_request($_GET['thread']); // get requested thread or default
$thread = $threads->getbyid($tid); // save thread in array
$permisions = new permisions(); // create permisions object
// check user permisions against thread permisions

$permisions->check_read($_SESSION['upermisions'],$thread['tpermisions']);

$template = new template($thread['ttemplate']); // create template object
$menu = new menu(); // create menu object
$main = $menu->main(); // save menu item(s) in array
// add [head] to template object

$template->parse('head',array('title' => APPLICATION_TITLE.' | '.$thread['ttitle']));

for ($i=0;$i < count($main);$i++) { // loop through menu array
$template->parse('menu',$main[$i]); // add [menu] to template object
}

$template->parse('content'); // add [content] to template object
// create articles object - paging is setup in the __construct

$articles = new articles($tid,$_GET['page'],$thread['tarticlesperpage']);

$article_array = $articles->get_articles(); // save article(s) in an array

for ($i=0;$i<count($article_array);$i++) { // loop through articles array
$template->parse('article',$article_array[$i]); // add [article] to template object
}

$paging = $articles->get_paging_links(); // save paging navigation into an array
// add [footer] to template object

$template->parse('footer',array('pagination' => $paging['prev'].$paging['next']));

$template->render(); // output the page to the client

?>


thankyou...

[pohopo]

IMHO,

I am guessing the two includes below are to initialize variables/constants and are not classes.
require_once('core/includes/application.config.php');
require_once('core/includes/functions.php');
Since both are required i would think you could make this one parent class and use the constructor method (along with accessor methods) to do the same thing. The rest of your includes would then be child classes (ie inherentence). With this you would not need to pass as many variables to your object as it can get the values from the parent class.

Another small thing I like to do is place an 'obj_' prefix in front of all objects as it makes things easier to read and understand.

[Shrike]

Originally posted by pohopo
IMHO,

I am guessing the two includes below are to initialize variables/constants and are not classes.
require_once('core/includes/application.config.php');
require_once('core/includes/functions.php');
Since both are required i would think you could make this one parent class and use the constructor method (along with accessor methods) to do the same thing. The rest of your includes would then be child classes (ie inherentence). With this you would not need to pass as many variables to your object as it can get the values from the parent class.

Another small thing I like to do is place an 'obj_' prefix in front of all objects as it makes things easier to read and understand.
I would actually recommend that you avoid this approach. Not because it's a bad idea, it's a good idea. But I've done this and often end up with many classes extending this one 'god' class. Use of static variables and the Singleton pattern would achieve the same thing, without having to worry about inheritance. If there are many different parts the Registry pattern might be useful too.

[pohopo]

I personally do not like functions and variables outside of classes as you lose reuse and control. Example, if you set $login_date to '20040201', without an accessor method, someone else that works on your project could change that to '02/01/2004' and possibly bring down your application. And if you do not inherent then you have to pass and define the common variables for each class. This is assuming you use accessor methods to protect and make sure you are getting variables in the format you want.

As for the 'god' class, this something you need to be careful with. If you are literally throwing everything in it then it defeats the purpose of OOP. I use inheretence all the time and it makes coding much easier for me. For it to work you do need to document before hand. Next to me is a diagram of all my parent and child classes that I use as a reference guide.

note: this is for PHP 5 as you are not able to encapsulate with PHP 4 making OOP hard to do.

disclaimer: I am new to PHP myself, so I do welcome any corrections in my above statements and I am far from an expert of PHP. Also, part of me is writing this for myself as I am also trying to learn the best way to code in PHP and this the best way I have figured out so far. Though I am always open for something better.

[LordShryku]

Just as a question, I see you passing GET and SESSION variables as arguments through some of your functions. Is this something that will always be superglobal or is there possibility of other variable types? Since they are superglobal, they should already be within the scope of the function, and shouldn't need to be passed.

[thorpe]

thanks for your input people. im a little confused with patterns and stuff having never eally done any oop before. ive only been programming for 18 months, and i started with asp. vbscript has classes, but nowhere near the features of php.

anyway. i had been told by someone on this board a while back that i should only really relate objects that should relate. ie: have your 'dog' class inherit from your 'animal' class. when i started this app, i could'nt really see how any of the objects related, but now i can see that some do, and some should.

the ones that stand out in particular are the 'threads' and 'articles' classes. bassicaly, each thread is a section of the site (eg: home, about....), and of course the aricles are the content for each thread. pushing it even further, both these objects could (should?) somehow be related to the template class as this actually generates there output.

im just not sure how to glue this all together. your comments have been a great help, and have led me to find a new site for some ideas. (phppatterns)

im gonna have a bit of a read over there and see if i can come up with (or if i need) another approuch.

one more question. im a little concerned about some of these objects returning arrays. i meen, it works, i just dont feel comfortable with the idea, and think i should return objects instead. i realise its probably just personal preference, but any opinions would be welcomed.

thanks again.

ps: LordShryku. im not 100% sure why i did such things with globals. guess i just wasn't thinking. hehe. i'll clean that up when i get home.

[pohopo]

I am not sure you should try to tackle design patterns till you better understand objects and inheretence. With Patterns you get into more complicated OOP where you use interface classes. It is powerful, but it is a lot of work and meant more for larger projects.

Have you read up on UML at all? I would create a Static class structure of all your classes. This will give you global view of what you have and make it easier to understand objects. I would do this before reading up on design patterns.

[thorpe]

cool... little steps are good. i have read a little on uml, but not much.

you know where i might find a tutorial or some more info on creating this static class structure?

[pohopo]

this will link you to a lot of good UML resources. http://www.uml.org/

UML deals with every part of a project, for now you want to look subjects that deal with Class diagrams. Though, it is good to understand all of UML. They also have program specific to building UML diagrams.

[Shrike]

Originally posted by pohopo
As for the 'god' class, this something you need to be careful with. If you are literally throwing everything in it then it defeats the purpose of OOP. I use inheretence all the time and it makes coding much easier for me. For it to work you do need to document before hand. Next to me is a diagram of all my parent and child classes that I use as a reference guide.
Imagine this situation though:

class Threads extends Config {}
class Articles extends Config {}
class Users extends Config{}
$threads = new Threads;
$articles = new Articles;
$users = new Users;

Here we now have 3 copies of the Config class when we really only wanted one. Inheritance should be used to specialise a more general class.

[pohopo]

class Threads extends Config {}
class Articles extends Config {}
class Users extends Config{}
$threads = new Threads;
$articles = new Articles;
$users = new Users;


You are right. The Config class should be on its own. However if Threads is part of Articles you could make Articles an extended class.

Another question. Say you created your $Config object and then you create an object for $Articles where many of the methods in $Articles use methods in $Config. Is there a way to use $Config methods in $Articles without defining 'global $Config' in each function?

Hope that makes sense :queasy:

[thorpe]

Is there a way to use $Config methods in $Articles without defining 'global $Config' in each function?

do you meen if $config is the parent of $articles? example:

parent::methodname();

[pohopo]

no, if $config is not the parent of $Articles. Currently I have to use global in each method of $Articles to access $config. I was if there was a shorter way to access $config without needed to include

global $config

every time.

[Shrike]

In all liklihood the config object wouldn't actually need to be an object so you could use the class statically (e.g. Classname::method() ) however if you wanted an object then the Singleton design pattern/static variable approach would work. In PHP 4 only functions can have static variables so a minor hack is required.

class ConfigSingleton
{
var $somekey = "test";
// A pretend method - does something useful
function getParameter($key)
{
if(isset($this->$key))
{
return $this->$key;
}
}

function setParameter($key, $value)
{
if(isset($this->$key))
{
$this->$key = $value;
}
}
// This creates a static instance of the class
function getInstance()
{
static $instance;
if(!isset($instance))
{
$instance = new ConfigSingleton;
}
return $instance;
}
}
//Usage - has no scoping problem as we use the static calling syntax
$object = ConfigSingleton::getInstance();
echo $value = $object->getParameter('somekey');
$object->setParameter('somekey', 'test2');
$object2 = ConfigSingleton::getInstance();
echo $object2->getParameter('somekey');

This works a bit better in PHP 5 due to class static variables and static methods.

I've edited the above to fix a bug and illustrate the usefulness of the pattern a little more :)