4
\$\begingroup\$

I am learning some OOP on PHP and I have developed some sample code to practise.

Ignoring the functionality, I just want to learn the "best" or "better" way to write OOP codes in PHP.

The following is my code and I hope anyone who has been doing PHP can help to review or comment for any improvements or weaknesses on my code.

I want to develop my style of code in which copy-and paste is minimum and functionality expandability is rich. For example, the execute() function in the UserDB class is basically the same function and can be used widespread in many diff db class. Do I need to copy and paste the code again or should I just moved this piece of code to abstract class?

How can I optimized my code to made it better for future functionalities such as other select, insert, update and delete while maintaining readability and still within OOP standards?

<?php

    require_once "sys_config.php";
    require_once "general.inc";
    $para[":user"]= "%";

    $dataset = UserFactory::getUsers($para);

    var_dump($dataset);

?>

UserFactory class

<?php

    class UserFactory{

        public static function getUsers($para)
        {
            $db = new UserDB("CALL getUsers(:user)",$para); //sp

            $objs = $db->execute();
            if(!empty($objs))
                foreach( $objs as $obj)
                {
                    $users[] = new user($obj);
                }
            return $users;
        }

    }


?>

UserDB class

<?php

    Class UserDB extends Database{

        protected $conn;
        protected $SQL;
        protected $para;

        function __construct($SQL, $para)
        {
            parent::__construct();

            $this->conn = parent::getConn();
            $this->SQL = $SQL;
            $this->para = $para;
        }

        function execute()
        {

            $query = $this->conn->prepare($this->SQL);
            if(!empty($this->para))
                foreach($this->para as $key=>$val)
                {
                    $query->bindParam($key,$val);
                }
            $results = $query->execute();

            $dataset = null;

            if($results)
            {
                while($row = $query->fetch(PDO::FETCH_OBJ))
                {
                    $dataset[]=$row;
                }
            }   

            unset($this->conn); 

            parent::__destruct();

            return $dataset;

        }

    }

?>

Database abstract class

<?php

    abstract Class Database{

        protected $conn;

        function __construct()
        {
            $this->conn = new PDO ("mysql:host=".DB_LOCATION.";dbname=".DB_NAME,DB_USER,DB_PASS);

        }

        function __destruct()
        {
            unset($this->conn);

        }

        function getConn()
        {
            return $this->conn;
        }

        abstract function execute();
    }

?>

User object class

<?php

    class User{

        private $fullname;
        private $id;

        function __construct($userObj)
        {
            $this->fullname = $userObj->fullName;
            $this->id = $userObj->id;
        }

        function setFullname($fullname)
        {
            $this->fullname = $fullname;
        }

        function getFullname()
        {
            return $this->fullname;
        }

        function setId($id)
        {
            $this->id = $id;
        }

        function getId()
        {
            return $this->id;
        }
    }

?>
\$\endgroup\$
3
  • \$\begingroup\$ Melvin the code in itself looks good. What you are doing, not so much. Ignoring the functionality, I just want to learn the "best" or "better" way to write OOP codes in PHP doesn't work well on Code Review, everything you post is fair game for reviewers. But it would help if you tell us a bit about the purpose of the classes. Not in excruciating detail, but just a little bit so we won't have to guess. \$\endgroup\$
    – yannis
    Commented Feb 10, 2012 at 4:59
  • \$\begingroup\$ I want to develop my style of code in which copy-and paste is minimum and functionality expandability is rich. for eg : the execute() function in UserDB class..basically the same function can be used widespread in many diff db class, so do I need to copy and paste the code again or should I just moved this piece of code to abstract class ? \$\endgroup\$
    – Melvin
    Commented Feb 10, 2012 at 6:01
  • \$\begingroup\$ Edit the question and add clarifications there, not in comments. I've favourited the question to not forget about and will come back for a proper review, hopefully soon (work is crazy, so, not a promise). \$\endgroup\$
    – yannis
    Commented Feb 10, 2012 at 6:09

1 Answer 1

2
\$\begingroup\$

Just a few comments on the code as it stands so far:

  1. You're using pretty old-school file dependency management (which isn't the same as dependency management) by explicitly including things. I'd strongly suggest you look into the SPL autoload system, write an autoloader that can find where your classes are located in the filesystem and rely on the autoloader to handle the file dependency management instead. It means that only code that's actually going to be used will be included and you won't get into a situation where you have loads of included classes but only a few get used, or where you're not sure what's been included.
  2. You're using static methods. Generally, statics are frowned upon because they're not considered to be good OO practice. Sometimes they're necessary, but as a rule you should avoid them. Any other class that makes use of the static call becomes tightly coupled to the class that implements it, reducing re-usability and testability (it becomes very difficult to test the dependant modules in isolation because you can't easily replace the static with a mock implementation). I'd advise refactoring your factory to work in a non-static way.
  3. Your Database class seems like it might not be necessary. It's merely a wrapper around PDO, and actually hides most of the PDO class functionality. I'd suggest you might be better off using PDO directly, as it's got a well-documented interface and you'll have full access to all the PDO functionality in the consuming classes instead of the small subset your Database class provides.

I would say that on the whole you're on basically the right track though. It's certainly better than my own early efforts with OOP. Don't worry, thinking in an "objecty" way is something that comes with practice and experience.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.