SitePoint Sponsor

User Tag List

Results 1 to 6 of 6
  1. #1
    Non-Member coo_t2's Avatar
    Join Date
    Feb 2003
    Location
    Dog Street
    Posts
    1,819
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)

    comment on OOP novice's PHP 5 code

    comment on OOP novice's PHP 5 code

    Hey all. Well, here are the first set of classes that I've written after
    having somewhat of a clue about what OOP is about.
    I mean I've done one or two classes before, but I really didn't know what the hell I was doing. Maybe I still dont? But I have read a book and
    think I understand the basic concepts; inheritence, encapsulation, polymorphism.

    So I wanted to see if you guys think I'm adhering to the fundamental principles
    of OOP here.
    This code is not meant to work(for now). I haven't been running it, I've just been coding it. It's more about trying to get the design right, learning how objects
    work on their own and with each other, etc..

    I'd like to have any general comments about the code to let me know if you
    think I'm on the right track.
    I'd also appreciate comments on some specifics.

    1. The "MyUser" class. This class really just wraps functionality from other
    classes(composition). The thing is, I think because it uses composition in this
    manner that it loses some usability. I mean depending on the situation I might want to use a different subclass of UserAccess, or UserRegistration, etc..
    I shouldn't have to use a different subclass of User for every possible combination? One idea I have is to pass the classes to be used to the constructor of "MyUser". Is that an acceptable approach? It does make the object more reusable in my opinion. Maybe it's bad design to start with? Maybe it isn't bad design but there's a clever way(other than my idea) to make it more
    reusable.

    2. Error handling. Should I give every class public methods for getting error
    information if there's a possibility that something could go wrong during the use of the object? For instance the "logIn()" method in "MyUser"?
    If "logIn()" fails should I set some attributes to report the errors and make
    them accessible through public methods? Or should I return an error Object like PEAR? Or maybe there's another alternative?
    Should I create one Error class that all my other classes can use or inherit from? Or is it ok to have classes handle their own errors their own way?


    I'm sorry this is such a huge post. But I thought it was important to post
    all the code so you guys could see how I'm trying to use inheritence and polymorphism. If you guys say I've done fairly well on this I will be encouraged and maybe start writing all my new code the OO way. If you say I still suck
    then I'll get some more knowledge before I start churning out bad OO code.
    Besides why churn out bad OO code when I'm already great at churning out bad
    procedural code


    Here goes....

    PHP Code:
    <?php

    error_reporting
    (E_ALL);

    abstract class 
    User {

        abstract function 
    __construct($dbObj);

        
    /* returns: bool */
        
    public abstract function logIn();

        
    /* returns: bool */
        
    public abstract function logOut();
        
        
    /* returns: bool */
        
    public abstract function register();

        
    /* returns: bool*/
        
    public abstract function isRegistered();

        
    /* returns: bool */
        
    public abstract function isLoggedIn();

        
    /* returns: bool */
        
    public abstract function setAccessLevel();

        
    /* returns: mixed */
        
    public abstract function getAccessLevel();
    }


    class 
    MyUser extends User {

        
    /* type: Object(REGISTRATION) */
        
    private $Registration;
        
        
    /* type: Object(UserLogin) */
        
    private $MyUserAccess;

        
    /* type: Object(Eclipse::MyDatabase) */
        
    private $dbObj;

        
    /* type: mixed */
        
    private $userLevel;

        function 
    __construct($dbObj)
        {   
            
    $this->dbObj $dbObj;
            
    $this->RegistrationObj null;
            
    $this->MyUserAccessObj null;
        }

        
    /* returns: bool */
        
    public function logIn($userName$passWord)
        {   
            if ( isset(
    $this->MyUserAccessObj) && $this->MyUserAccessObj->isLoggedIn() )
            {   
                echo 
    "Error: You're already logged in!!\n\n";
                echo 
    "Error trace back: \n\n";
                
    print_r(debug_backtrace());
                die;
            }
            elseif (!isset(
    $this->MyUserAccessObj) )
            {   
                
    $this->MyUserAccessObj = new MyUserAccess($this->dbObj$userName$passWord);

                return 
    $this->MyUserAccessObj->logIn($userName$passWord);
            }
        }

        
    /* returns: bool */
        
    public function logOut()
        {   
            return 
    $this->MyUserAccessObj->logOut();
        }
        
        
    /* returns: bool */
        
    public function register()
        {   
            if (!isset(
    $this->RegistrationObj) )
            {   
    $this->RegistrationObj = new MyUserRegistration($this->dbObj);
            }

            if ( !
    $this->RegistrationObj->isRegistered() )
            {   return 
    $this->RegistrationObj->register();
            }
            else
            {   echo 
    "Error: Can't register twice!!\n\n";
                echo 
    "Error trace back: \n\n";
                
    print_r(debug_backtrace());
                die;
            }
        }

        
    /* returns: bool*/
        
    public function isRegistered()
        {   return ( isset(
    $this->RegistrationObj
                    && 
    $this->RegistrationObj->isRegistered() );
        }

        
    /* returns: bool */
        
    public function isLoggedIn()
        {   return ( isset(
    $this->MyUserAccessObj) && $this->MyUserAccessObj->isLoggedIn() );
        }

        
    /* returns: bool */
        
    public function setAccessLevel($level)
        {   
            if ( !isset(
    $this->MyUserAccessObj) )
            {   
    $this->MyUserAccessObj = new MyUserAccess($this->dbObj$userName$passWord);
            }

            return 
    $this->MyUserAccessObj->setAccessLevel($level);
        }

        
    /* returns: mixed */
        
    public function getaccessLevel()
        {   return 
    $this->MyUserAccessObj->getAccessLevel();
        }
    }


    abstract class 
    UserAccess {

        
    /* type: string */
        
    private $userName;
        
        
    /* type: string */
        
    private $passWord;

        
    /* type: bool */
        
    private $isLoggedIn;
        
        
    /* type: mixed */
        
    private $accessLevel;

        
    /* type: Object */
        
    private $dbObj;
        
        function 
    __construct($dbObj$userName$passWord)
        {   
            
    $this->dbObj $dbObj;
            
    $this->userName $userName;
            
    $this->passWord $passWord;
            
    $this->isLoggedIn false;
            
    $this->accessLevel null;
        }

        
    /* returns: bool 
           The actual implementation in the extended 
           class will probably do something such as 
           check that the user is valid and set a session 
           variable or cookie value. */
        
    public abstract function logIn();

        
    /* returns: bool
           The implementation will probably do something 
           like set a $_SESSION['loggedIn'] variable to false 
           or set a cookie value to false.  */
        
    public abstract function logOut();

        
    /* returns: bool */
        
    public abstract function isLoggedIn();

        
    /* returns: bool */
        
    public abstract function setUserName();

        
    /* returns: bool */
        
    public abstract function setPassword();

        
    /* returns: bool */
        
    public abstract function setAccessLevel();

        
    /* returns: string */
        
    public abstract function getUserName();

        
    /* returns: string */
        
    public abstract function getPassword();

        
    /* returns: mixed */
        
    public abstract function getAccessLevel();
    }


    class 
    MyUserAccess extends UserAccess {

        
    /* type: object */
        
    private $MysqlStatementObj;

        function 
    __construct($dbObj$userName$passWord)
        {   
            
    parent::__construct();

            
    $this->MysqlStatementObj null;
        }

        
        private function 
    checkUser()
        {
            
    $sql 'SELECT COUNT(id) FROM user WHERE username = ? AND password = ?';
            
            
    /* MySqlStatement is a modified version of the class 
               created here [url]http://sitepointforums.com/showthread.php?threadid=95763[/url] ..
               The modified version returns a MyQueryResult object 
               from the Eclipse library .
             */
            
    $MysqlStatementObj = new MySqlStatement($sql$this->dbObj);

            
    $MysqlStatementObj->setParameter(1$this->userName);
            
    $MysqlStatementObj->setParameter(2$this->passWord);
            
    $MyQueryResultObj $MysqlStatementObj->execute();

            return (
    $MyQueryResultObj->getRowCount() == 1);
        }


        public function 
    logIn()
        {   
            
    #echo "MyUserAccess::logIn returning true\n";

            
    if ( $this->checkUser() )
            {   
    $this->isLoggedIn true;
                return 
    true;
            }
            else
            {   
    /* 
                Put some error handling here.
                What should I do? 
                Instantiate an error reporting class of some type?
                If I return an object then the client has to be ready 
                to catch either a value of TRUE or an Error Object.
                */
                
    return false;
            }
        }

        public function 
    logOut()
        {   
    #echo "MyUserAccess::logOut returning true\n";
            
    $this->isLoggedIn false;
            return 
    true;
        }

        public function 
    isLoggedIn()
        {   return 
    $this->isLoggedIn;
        }

        public function 
    setUserName($newName)
        {   
            if (!
    $this->isLoggedIn() )
            {   die(
    "You have to log in to change your username\n");
            }

            
    $sql 'UPDATE user SET username = ? WHERE username = ? AND password = ?';
            
            
    $MysqlStatementObj = new MySqlStatement($sql$this->dbObj);

            
    $MysqlStatementObj->setParameter(1$newName);
            
    $MysqlStatementObj->setParameter(2$this->userName);
            
    $MysqlStatementObj->setParameter(3$this->passWord);
            
    $MyQueryResultObj $MysqlStatementObj->execute();

            if ( 
    $MyQueryResultObj->isSuccess() )
            {   
    $this->userName $newName
                return 
    true;
            }
            else
            {   
    /*
                Again I'm not sure of the best way to do 
                error handling */
                
    return false;
            }
        }

        public function 
    setPassword($newPassword)
        {   
            if (!
    $this->isLoggedIn() )
            {   die(
    "You have to log in to change your password\n");
            }

            
    $sql 'UPDATE user SET password = ? WHERE username = ? AND password = ?';
            
            
    $MysqlStatementObj = new MySqlStatement($sql$this->dbObj);

            
    $MysqlStatementObj->setParameter(1$newPassword);
            
    $MysqlStatementObj->setParameter(2$this->userName);
            
    $MysqlStatementObj->setParameter(3$this->passWord);
            
    $MyQueryResultObj $MysqlStatementObj->execute();

            if ( 
    $MyQueryResultObj->isSuccess() )
            {   
    $this->passWord $newPassword
                return 
    true;
            }
            else
            {   
    /*
                Again I'm not sure of the best way to do 
                error handling */
                
    return false;
            }
        }


       
    /* not reaally implemented yet */
        
    public function setAccessLevel($level)
        {   
            if (!
    $this->isLoggedIn() )
            {   die(
    "You have to log in to change accessLevel.\n");
            }

            
    $this->accessLevel $level;
            return 
    true;
        }

        public function 
    getUserName()
        {   return 
    $this->userName;
        }

        public function 
    getPassword()
        {   return 
    $this->passWord;
        }

        public function 
    getAccessLevel()
        {   return 
    $this->accessLevel;
        }
    }


    abstract Class 
    UserRegistration {

        
    /* I have no idea what this 
           abstract class(not to mention the implementation) 
           should look like since the registration 
           process can vary so much from one app to 
           the other. */

        
    private $isRegistered;
        
        
    /* this will probably be handled by a 
           separate class */
        
    abstract function validateUserInput();

        
    /* returns: bool */
        
    abstract function insertUserInfo();
    }

    // Using eclipse, will change things to be PHP 5 compatible
    $dbObj = new MyDatabase($dbName$dbHost); 
    $dbObj->connect($dbUser$dbPass); 

    $objUser = new MyUser($dbObj);

    if ( 
    $objUser->logIn('ed''sped') )
    {   
    $_SESSION['loggedIn'] = true;
    }
    else
    {  echo 
    'login failed<br>';
       
    /* Should $objUser contain some methods for getting the error 
          info ? */
    }

    $objUser->setAccessLevel(1);
    echo 
    $objUser->getAccessLevel();

    $objUser->logOut();

    $dbObj->disconnect();

    ?>

  2. #2
    SitePoint Member
    Join Date
    Feb 2003
    Location
    Groningen, Netherlands
    Posts
    21
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    2)
    I think the best way to handle errors in oop-code is throwing exceptions.
    In the logIn-method you could throw an exception. And catch it outside the method:


    PHP Code:
    <?
    try
    {
        
    $dbObj = new MyDatabase($dbName$dbHost);
        
    $dbObj->connect($dbUser$dbPass); // throws DBException

        
    $objUser = new MyUser($dbObj);

        
    $objUser->logIn('ed''sped') )  // throws LoginException
        
    $_SESSION['loggedIn'] = true;
    }
    catch (
    DBException $e
    {   echo 
    'database error<br>';
        echo 
    $e->getmessage();

        
    // here you could decide to rethrow the exception
        // throw new Exception('...');
    }
    catch (
    LoginException $e
    {   echo 
    'login failed<br>';
        echo 
    $e->getmessage();

        
    // here you could decide to rethrow the exception
        // throw new Exception('...');
    }
    ?>

  3. #3
    SitePoint Zealot
    Join Date
    Aug 2002
    Posts
    178
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Yeah: use trow/catch and use a interface for your User class instead of a abstract class, then you can still extend something else AND implement the User interface at the same time.
    I assume you're not using these yet, but: don't echo out stuff inside the class; set an error, then check for that and do stuff based on the state of the object.
    (and the "die()" stuff is for development I assume??)

  4. #4
    Non-Member coo_t2's Avatar
    Join Date
    Feb 2003
    Location
    Dog Street
    Posts
    1,819
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by nucleuz
    Yeah: use trow/catch and use a interface for your User class instead of a abstract class, then you can still extend something else AND implement the User interface at the same time.

    Yeah I was gonna ask whether User should be an interface.
    At one point I did make it an interface but then switched and made
    it an abstract class. I don't know don't why I did that, it seemed
    like it should of been an interface.( hears obi-wan's faint voice saying
    "trust your feelings coo_t2, the force will not deceive you.")


    Quote Originally Posted by nucleuz
    I assume you're not using these yet, but: don't echo out stuff inside the class; set an error, then check for that and do stuff based on the state of the object.
    (and the "die()" stuff is for development I assume??)
    I like the idea of throwing an exception in the class(as you and sjokki have suggested) if something goes
    wrong because doing it that keeps to minimum the amount of error handling code
    I actually have to put in each class.

    I just put the die() stuff there because I wasn't sure how to actually go about it. And at first I was running the code so I just had it die when it
    reached those points.

    --ed

  5. #5
    public static void brain Gybbyl's Avatar
    Join Date
    Jun 2002
    Location
    Montana, USA
    Posts
    647
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Yea, definately go with the interface on the user class, since its nothing but prototypes -- the only time i use abstract classes is if i need to define some sort of functionality or define member variables (something that can't be done in an interface).

    Even though it's already been said, don't use echo or die statements in your classes, that should be left for the implementation (controller component) of your class. I would even go so far as to seperate your database connectivity/driver from your login class. That will just be another small little step towards seperation of code and design.
    Ryan

  6. #6
    Non-Member coo_t2's Avatar
    Join Date
    Feb 2003
    Location
    Dog Street
    Posts
    1,819
    Mentioned
    1 Post(s)
    Tagged
    1 Thread(s)
    Quote Originally Posted by Gybbyl
    I would even go so far as to seperate your database connectivity/driver from your login class. That will just be another small little step towards seperation of code and design.
    Not sure what you mean?

    --ed


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •