Page 1 of 1

ShoutBox data manager.. I think it needs review.

Posted: Tue Jun 16, 2009 10:41 am
by aptfort

Code: Select all

<?php
 
class ShoutboxManager
{
    private $_DB;
    private $_shoutbox; // List
    private $_currentPost; // Single post
    
    public function __construct($dbLink)
    {
        $this->_DB = $dbLink;
    }
    
    public function get_MostRecent()
    {
        // single Query and fetch
        $shoutFetch = $this->_DB->QFetch( "SELECT * FROM `shoutbox` ORDER BY `timestamp` DESC LIMIT 1" );
        $this->_currentPost = $this->_create_Shout();
        $this->_currentPost->conjoin($shoutFetch);
        
        return $this->_currentPost;
    }
    
    private function _create_Shout()
    {
        return new ShoutboxPost();
    }
    
    private function _create_Shoutbox()
    {
        return new Shoutbox();
    }
    
    private function populate_Shoutbox($query)
    {
        //TODO: Somewhere in this code, we have to check if we have a shoutbox created
        while( $shoutFetch = $this->_DB->Fetch($query) )
        {
            $tempPost = $this->_create_Shout();
            
            $tempPost->conjoin($shoutFetch);
            
            $this->_shoutbox->add_Post($tempPost);
        }
        unset($tempPost);
    }
}
 
 
class Shoutbox
{
    private $_list;
    
    public function __construct()
    {
        $this->_list = array();
    }
    
    public function get_List()
    {
        return $this->_list;
    }
    
    public function add_Post(ShoutboxPost $shout)
    {
        $this->_list[] = $shout;
    }
}
 
class ShoutboxPost
{
    private $_postId;
    private $_charId;
    private $_charName;
    private $_shoutPost;
    private $_timestamp;
    
    public function __construct()
    {
        // Initializers
        $this->_postId = 0;
        $this->_charId = 0;
        $this->_charName = "";
        $this->_shoutPost = "";
        $this->_timestamp = 0;
    }
    
    public function __get($var)
    {
        $tempvar = "_$var";
        return $this->$tempvar;
    }
    
    public function __set($var, $value)
    {
        switch($var)
        {
            case "Id":
                $this->_set_PostId($value);
            break;
            case "charId":
                $this->_set_CharId($value);
            break;
            case "charName":
                $this->_set_CharName($value);
            break;
            case "shoutPost":
                $this->_set_ShoutPost($value);
            break;
            case "timestamp":
                $this->_set_Timestamp($value);
            break;
            default:
            return;
        }
    }
    
    public function conjoin( $fetch )
    {
        $this->_set_PostId($fetch['id']);
        $this->_set_CharId($fetch['charId']);
        $this->_set_CharName($fetch['charName']);
        $this->_set_ShoutPost($fetch['shoutPost']);
        $this->_set_Timestamp($fetch['timestamp']);
    }
    
    private function _set_PostId($id)
    {
        $this->_postId = $id;
    }
    
    private function _set_CharId($id)
    {
        $this->_charId = $id;   
    }
    
    private function _set_CharName($name)
    {
        $this->_charName = $name;
    }
    
    private function _set_ShoutPost($shoutPost)
    {
        $this->_shoutPost = $shoutPost;
    }
    
    private function _set_Timestamp($timestamp)
    {
        $this->_timestamp = $timestamp;
    }
}
?>
Hey all, this is my first time posting here. This is probably the first time I've really looked at php forums this year. Well, I am requesting a review of this OOP code. So far, I've made a 'manager', a list, and the object itself. The 'manager' gets the data from the database, creates the shout objects, populates the objects with db data, and adds them to its list.

Personally, I think I am overusing objects. The manager works of course, but this is much easier procedurally. I am thinking of scaling this class file down, but I am not sure how to proceed yet. I have other 'managers' that follow this little paradigm. Whether or not this is good is a personal preference, but I'd like opinions. Thanks.

Also, anything that needs explaining, just ask. Everything is self explanatory to me and should be self explanatory in general, but just in case just ask.

Re: ShoutBox data manager.. I think it needs review.

Posted: Wed Jun 17, 2009 4:50 am
by aptfort
I was thinking I would create a 'datamanager' who would have access to the database, while a 'manager' would request a 'datamanager' to grab data that it needs in order to create shout objects. I would remove the list object in favor of the manager having an array of shout object.

Second way, I would remove the shout object and keep only a 'manager' and a 'datamanager', which is probably not a good idea since the shout object is a data collection object.

Lastly, just get rid of the whole OOP paradigm and go with a procedural of doing this, which is unlikely because I could use this in my next project.

Re: ShoutBox data manager.. I think it needs review.

Posted: Wed Jun 17, 2009 4:57 am
by onion2k
Have you tested your code?

It's just that you have a function called _create_Shootbox()...

Re: ShoutBox data manager.. I think it needs review.

Posted: Wed Jun 17, 2009 5:36 am
by aptfort
onion2k wrote:Have you tested your code?

It's just that you have a function called _create_Shootbox()...
Yes I have, but that function is there for later. Misspelling, of course. Probably not needed. Thanks for that notice.