ShoutBox data manager.. I think it needs review.

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
aptfort
Forum Newbie
Posts: 3
Joined: Sat Jun 13, 2009 6:36 am
Location: Anywhere, but here.

ShoutBox data manager.. I think it needs review.

Post 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.
Last edited by Benjamin on Tue Jun 23, 2009 10:14 am, edited 2 times in total.
Reason: Changed code type from text to php.
aptfort
Forum Newbie
Posts: 3
Joined: Sat Jun 13, 2009 6:36 am
Location: Anywhere, but here.

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

Post 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.
User avatar
onion2k
Jedi Mod
Posts: 5263
Joined: Tue Dec 21, 2004 5:03 pm
Location: usrlab.com

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

Post by onion2k »

Have you tested your code?

It's just that you have a function called _create_Shootbox()...
aptfort
Forum Newbie
Posts: 3
Joined: Sat Jun 13, 2009 6:36 am
Location: Anywhere, but here.

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

Post 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.
Post Reply