SitePoint Sponsor

User Tag List

Page 1 of 2 12 LastLast
Results 1 to 25 of 41
  1. #1
    SitePoint Enthusiast
    Join Date
    Nov 2008
    Posts
    73
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Very basic OOP feedback

    Hi, I am in the process of making the switch from procedural to OO PHP. I'm just practising by setting up really simple config, database and auth classes. Before I end up going too far down the road with dodgy code, i would be interested to get some feedback on what i've done so far.

    One thing I'm not sure about - where to start sessions, i.e in the auth class file in the constructor or outside of the class (but in the same file...if that makes sense).

    Thanks in advance for any advice!

    config.php
    PHP Code:
    class Config {
        
        protected 
    $hostname 'localhost';
        protected 
    $username 'root';
        protected 
    $password '';
        protected 
    $database 'my_database';
        
      
    /**
       * Config::get_hostname()
       * 
       * @return
       */
        
    public function get_hostname()
        {
            return 
    $this->hostname;
        }
        
      
    /**
       * Config::get_username()
       * 
       * @return
       */
        
    public function get_username()
        {
            return 
    $this->username;
        }
        
      
    /**
       * Config::get_password()
       * 
       * @return
       */
        
    public function get_password()
        {
            return 
    $this->password;
        }
        
      
    /**
       * Config::get_database()
       * 
       * @return
       */
        
    public function get_database()
        {
            return 
    $this->database;
        }

    database.php
    PHP Code:
    class Database
    {
        private 
    $connectlink//Database Connection Link
        
    private $username;
        private 
    $password;
        private 
    $database;
        private 
    $hostname;
        private 
    $resultlink//Database Result Recordset link
        
    private $rows//Stores the rows for the resultset

      /**
       * Database::__construct()
       * 
       * @param mixed $conf
       * @return
       */
        
    public function __construct(Config $conf)
        {
            
    $this−>hostname = $conf->get_hostname();
            
    $this−>username = $conf->get_username();
            
    $this−>password = $conf->get_password();
            
    $this−>database = $conf->get_database();
            
            
    $this->connectlink mysql_connect($this->hostname$this->username$this->
                
    password);
            if ( ! (
    $this->connectlink))
            {
                throw new 
    Exception("Error Connecting to the Database");
            } else
            {
                
    mysql_select_db($this->database);
            }
        }

      
    /**
       * Database::__destruct()
       * 
       * @return
       */
        
    public function __destruct()
        {
            @
    mysql_close($this->connectlink);
        }

      
    /**
       * Database::query()
       * 
       * @param mixed $sql
       * @return
       */
        
    public function query($sql)
        {
            
    $this->resultlink mysql_query($sql);
            return 
    $this->resultlink;
        }

      
    /**
       * Database::fetch_rows()
       * 
       * @param mixed $result
       * @return
       */
        
    public function fetch_rows($result)
        {
            
    $rows = array();
            if (
    $result)
            {
                while (
    $row mysql_fetch_array($result))
                {
                    
    $rows[] = $row;
                }
            } else
            {
                throw new 
    Exception("Error Retrieving Records");
                
    $rows null;
            }
            return 
    $rows;
        }

    auth.php
    PHP Code:
    include("database.php");

    class 
    Auth {
        
        protected 
    $db;
        
      
    /**
       * Auth::__construct()
       * 
       * @return
       */
        
    public function __construct()
        {
            
    $db = new Database;
        }
        
        
        
    //function to check login credentials
      /**
       * Auth::login()
       * 
       * @param mixed $username
       * @param mixed $password
       * @return
       */
        
    public function login($username$password)
        {
            
    $query $db->query("SELECT username, password FROM users WHERE username = '".mysql_real_escape_string($username)."' AND password = sha1('".mysql_real_escape_string($password)."')");
            
    $result $db->fetch_rows($query);
            
            if (
    count($result) == 0)
            {
                return 
    FALSE;
            }
            else
            {
                
    //set session variable 
                
    $_SESSION['logged_in'] = TRUE;
                return 
    TRUE;
            }
        }
        
        
    //logout function
      /**
       * Auth::logout()
       * 
       * @return
       */
        
    public function logout()
        {
            return 
    $_SESSION['logged_in'] = FALSE;
        }
        


  2. #2
    SitePoint Guru
    Join Date
    Jan 2005
    Location
    heaven
    Posts
    953
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Mmm, I would probably not do this
    PHP Code:
            $this->hostname $conf->get_hostname();
            
    $this->username $conf->get_username();
            
    $this->password $conf->get_password();
            
    $this->database $conf->get_database(); 
    I would prefer to store the config object and use it's methods to access the data. Also, I would invoke __get and __set to allow read/write privileges on member variables rather than creating a getter and setter function for each variable.

    In the Auth class, rather than defining the db connection, I would pass it as a none required parameter in construct as well as create a setter and getter function for it. Login would then check to see if db connection is set and if not throw an error.
    Creativity knows no other restraint than the
    confines of a small mind.
    - Me
    Geekly Humor
    Oh baby! Check out the design patterns on that framework!

  3. #3
    SitePoint Enthusiast
    Join Date
    Nov 2008
    Posts
    73
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Thanks for the reply imaginethis. Ok I see how using __get and __set would be quicker. So get rid of my getters (no setters at the moment) and just use something like this:
    PHP Code:
    public function __get($var){

            return 
    $this->$var;

    When you say you would prefer to store the config object and use its methods to access the data, can you please explain or show a small example? I'm not sure how to go about this exactly

  4. #4
    SitePoint Guru
    Join Date
    Jan 2005
    Location
    heaven
    Posts
    953
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by franko75 View Post
    Thanks for the reply imaginethis. Ok I see how using __get and __set would be quicker. So get rid of my getters (no setters at the moment) and just use something like this:
    PHP Code:
    public function __get($var){

            return 
    $this->$var;

    When you say you would prefer to store the config object and use its methods to access the data, can you please explain or show a small example? I'm not sure how to go about this exactly
    PHP Code:
    class Database

    {
        private 
    $_config null;
        private 
    $_link null;
        private 
    $_db null;

      
    /**
       * Database::__construct()
       * 
       * @param mixed $conf
       * @return
       */

        
    public function __construct(Config $conf)
        {
            
    $this->setConfig($conf)
                 ->
    _connect()
                 ->
    _selectDb();
        }

        public function 
    setConfig($conf)
        {
            
    $this->_config $conf;
            return 
    $this;
        }
         
        protected function 
    _connect()
        {
            if(!(
    $conf $this->_conf))
              throw new 
    Exception('Config Object not found');

            
    $this->_link mysql_connect($conf->hostname$conf->username$conf->password);
            return 
    $this;
        }

        protected function 
    _selectDb()
        {
            if(!(
    $conf $this->_conf))
              throw new 
    Exception('Config Object not found');

            if(!(
    $link $this->_link))
              throw new 
    Exception('Connection to server failed');

            
    $this->_db mysql_select_db($config->database$link);
        }


    Creativity knows no other restraint than the
    confines of a small mind.
    - Me
    Geekly Humor
    Oh baby! Check out the design patterns on that framework!

  5. #5
    SitePoint Wizard silver trophybronze trophy Cups's Avatar
    Join Date
    Oct 2006
    Location
    France, deep rural.
    Posts
    6,869
    Mentioned
    17 Post(s)
    Tagged
    1 Thread(s)
    One thing I'm not sure about - where to start sessions, i.e in the auth class file in the constructor or outside of the class (but in the same file...if that makes sense).
    I'd say make a small class that does that session work for you.

    Then you can optionally make a class which manages both your auth class and your session class.

    You should be able to use your session class in other projects, like a piece of lego.

  6. #6
    SitePoint Enthusiast
    Join Date
    Nov 2008
    Posts
    73
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    @cups - thanks for the advice, i will give this a go. Normally I would use a framework or CMS and their in built session libraries, but although what i'm trying to do here is mainly for practice, it's also for small projects where a framework isn't really necessary. It would be really useful to have my own small scale library which i can re-use.

    @imaginethis - cheers for showing how to store the config object and use it's methods from within the database class. I'm presuming single underscores are being used for private methods/variables?

    In the Auth class, rather than defining the db connection, I would pass it as a none required parameter in construct as well as create a setter and getter function for it. Login would then check to see if db connection is set and if not throw an error.
    Had a bash at this, is this what you mean? Presuming i should be using a getter and setting on this occasion and not magic methods.
    PHP Code:
    class Auth {
        
        protected 
    $_db;
        
      
    /**
       * Auth::__construct()
       * 
       * @return
       */
        
    public function __construct($db null)
        {
            
    $this->db $db->get_db();
        } 
    PHP Code:
    class Database
    {
        private 
    $_config null;
        private 
    $_link null;
        private 
    $_db null;
        
         
        public function 
    __construct(Config $conf)
        {
            
    $this->set_config($conf)
                ->
    _connect()
                ->
    _select_db();
        }
        
        public function 
    set_config($conf)
        {
            
    $this->_config $conf;
            
            return 
    $this;
        }
        
        protected function 
    _connect()
        {
            if ( ! (
    $conf $this->_config))
                throw new 
    Exception('Config Object not found');
                
            
    $this->_link mysql_connect($conf->hostname$conf->username$conf->password);
            
            return 
    $this;
        }
        
        protected function 
    _select_db()
        {
            if ( ! (
    $conf $this->_config))
                throw new 
    Exception('Config Object not found');
                
            if ( ! (
    $link $this->_link))
                throw new 
    Exception('Connection to server failed');
                
            
    $this->_db mysql_select_db($conf->database$link);
        }
       
        public function 
    get_db()
        {
            return 
    $this->_db;
        }
        
        public function 
    set_db(Database $db)
        {
            return 
    $this->_db $db;
        }


  7. #7
    hi galen's Avatar
    Join Date
    Jan 2006
    Location
    New Haven, CT
    Posts
    1,228
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    you should try using pdo instead of the mysql_* functions

  8. #8
    Use The Cloud
    Join Date
    Jan 2006
    Location
    Boise, ID
    Posts
    556
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by franko75 View Post
    I'm presuming single underscores are being used for private methods/variables?
    Using underscores for private methods/data is pretty much a standard now.

    In the Zend Coding Standard you also use underscores for protected methods and data.

    In the PEAR Coding Standards you do not use an underscore for protected methods and data.

    I would suggest for your own projects you pick one of these and try to follow it reasonably close. Not only are these designed to make your code easier to read and edit later, but if you ever work in a group project with other developers you'll be able to adapt and write code to a standard.
    Brad Hanson, Web Applications & Scalability Specialist
    ► Is your website outgrowing its current hosting solution?
    ► PM me for a FREE scalability consult!
    ► USA Based: Available by Phone, Skype, AIM, and E-mail.

  9. #9
    SitePoint Wizard silver trophybronze trophy Stormrider's Avatar
    Join Date
    Sep 2006
    Location
    Nottingham, UK
    Posts
    3,133
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    PHP 5 has pretty much removed the need for using _ though, you can just declare a variable as private or protected. I always thought using _ was a kind of indicator when using php 4, when all variables were public.

  10. #10
    SitePoint Guru
    Join Date
    Jan 2005
    Location
    heaven
    Posts
    953
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by franko75 View Post
    PHP Code:
        public function set_db(Database $db)
        {
            return 
    $this->_db $db;
        } 
    This is sort of pointless. You wouldn't want to set the database with an existing instance.

    PHP Code:
    $this->db $db->get_db(); 
    This is also pointless. By passing $db, you already have a reference to the object.

    PHP Code:
    class Auth {

        protected 
    $_db;

      
    /**
       * Auth::__construct()
       * 
       * @return
       */
        
    public function __construct($db null)
        {
            
    $this->db $db;
        } 

        public function 
    login($username$password)
        {
            
    $query $this->_db->query(...);
        } 
    Creativity knows no other restraint than the
    confines of a small mind.
    - Me
    Geekly Humor
    Oh baby! Check out the design patterns on that framework!

  11. #11
    SitePoint Guru
    Join Date
    Jan 2005
    Location
    heaven
    Posts
    953
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Stormrider View Post
    PHP 5 has pretty much removed the need for using _ though, you can just declare a variable as private or protected. I always thought using _ was a kind of indicator when using php 4, when all variables were public.
    It's useful to use underscores for private and protected class members. It's a subtle reminder that the the variable or function that you are using is private or protected without having to revisit the declaration to figure out if variables label. It's a good habit NOT to break.
    Creativity knows no other restraint than the
    confines of a small mind.
    - Me
    Geekly Humor
    Oh baby! Check out the design patterns on that framework!

  12. #12
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    This doesn't answer your question, but I thought you would like to know you can write:
    PHP Code:
    class Config
    {
        protected 
    $hostname 'localhost';
        protected 
    $username 'root';
        protected 
    $password '';
        protected 
    $database 'my_database';

    As:
    PHP Code:
    class Config
    {
        protected
            
    $hostname 'localhost',
            
    $username 'root',
            
    $password '',
            
    $database 'my_database';

    Less times you have to repeat public/protected/private for class/object vars.
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  13. #13
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by imaginethis View Post
    It's useful to use underscores for private and protected class members. It's a subtle reminder that the the variable or function that you are using is private or protected without having to revisit the declaration to figure out if variables label. It's a good habit NOT to break.
    True it is a helpful reminder, however, for me personally my class variables are always protected never public and never private. So its not much help :P
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  14. #14
    Use The Cloud
    Join Date
    Jan 2006
    Location
    Boise, ID
    Posts
    556
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by logic_earth View Post
    True it is a helpful reminder, however, for me personally my class variables are always protected never public and never private. So its not much help :P
    You use protected variables when declaring `final` classes?

    Brad Hanson, Web Applications & Scalability Specialist
    ► Is your website outgrowing its current hosting solution?
    ► PM me for a FREE scalability consult!
    ► USA Based: Available by Phone, Skype, AIM, and E-mail.

  15. #15
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    ^^^ I don't use final classes...ever
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  16. #16
    SitePoint Wizard silver trophybronze trophy Stormrider's Avatar
    Join Date
    Sep 2006
    Location
    Nottingham, UK
    Posts
    3,133
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Also, any decent editor will tell you what type of variable is being referred to as you type it, so it's not like you keep having to flick back to the declaration. I tend to remember which is which anyway, it's pretty obvious what should be private and what should be public.

  17. #17
    Use The Cloud
    Join Date
    Jan 2006
    Location
    Boise, ID
    Posts
    556
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Stormrider View Post
    Also, any decent editor will tell you what type of variable is being referred to as you type it, so it's not like you keep having to flick back to the declaration. I tend to remember which is which anyway, it's pretty obvious what should be private and what should be public.
    Obvious in your own code, but when reading code you've never seen before--anything to make it easier to work with is very welcome.

    What editor do you use?
    Brad Hanson, Web Applications & Scalability Specialist
    ► Is your website outgrowing its current hosting solution?
    ► PM me for a FREE scalability consult!
    ► USA Based: Available by Phone, Skype, AIM, and E-mail.

  18. #18
    SitePoint Wizard silver trophybronze trophy Stormrider's Avatar
    Join Date
    Sep 2006
    Location
    Nottingham, UK
    Posts
    3,133
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    NuSphere PHPEd. I used to use Zend, but it ran very slowly on my PC. PHPEd seems to have everything that I liked about Zend, plus more, and runs a lot faster as well (it isn't built in Java...).

    Surely if you are editing someone elses code, you have to know what the variable starts with anyway - if you are typing the variable name out, you already know if it's public or private because you have to know whether to start with a _ or not. If you don't know which it is, and DON'T use underscores, you can start typing the name and autocomplete will do the rest, and tell you what type it is

  19. #19
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by bhanson View Post
    Obvious in your own code, but when reading code you've never seen before--anything to make it easier to work with is very welcome.

    What editor do you use?
    Sure that is an issue...if the code is nothing but a mess and mixes variable visibilities.
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  20. #20
    Use The Cloud
    Join Date
    Jan 2006
    Location
    Boise, ID
    Posts
    556
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Stormrider View Post
    if you are typing the variable name out, you already know if it's public or private because you have to know whether to start with a _ or not.
    You know if it's public or private *because* you're using the underscore naming convention. If you aren't then you have no idea without looking at the definition.

    Quote Originally Posted by Stormrider View Post
    If you don't know which it is, and DON'T use underscores, you can start typing the name and autocomplete will do the rest, and tell you what type it is
    My vim doesn't tell me what type it is.

    Quote Originally Posted by logic_earth View Post
    Sure that is an issue...if the code is nothing but a mess and mixes variable visibilities.
    There are many "professional" PHP developers who write absolutely terrible code. At some point in your career you're bound to run into at least a few of them. However, your job becomes a lot easier if they were forced to use a coding standard that has an underscore naming convention.

    Even though *you* do not use private variables, other developers do and if you know what coding standard they're using then the code suddenly becomes that much more meaningful.

    Example:
    Programmer is using PEAR coding standard. You see variable $foo and know it is public or protected, you know automatically $_bar is private.

    Example 2:
    Programmer is using Zend coding standard. You see variable $foo and know it is public. You see $_bar and know it is protected or private.

    vs

    Example 3:
    Programmer is of unknown caliber and is not using a major coding standard. You see variable $foo and either 1. make an assumption or 2. look it up
    Brad Hanson, Web Applications & Scalability Specialist
    ► Is your website outgrowing its current hosting solution?
    ► PM me for a FREE scalability consult!
    ► USA Based: Available by Phone, Skype, AIM, and E-mail.

  21. #21
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    ^^^ In all three examples you have to look up all the vars anyways to know exactly what they are for, making the point moot. Why would one blindly use a variable one sees in the code?
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  22. #22
    SitePoint Wizard silver trophybronze trophy Stormrider's Avatar
    Join Date
    Sep 2006
    Location
    Nottingham, UK
    Posts
    3,133
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by bhanson View Post
    You know if it's public or private *because* you're using the underscore naming convention. If you aren't then you have no idea without looking at the definition.
    This doesn't make sense at all. You want to use a class variable, how do you know whether to use an underscore or not without finding its definition first?

  23. #23
    Use The Cloud
    Join Date
    Jan 2006
    Location
    Boise, ID
    Posts
    556
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Stormrider View Post
    This doesn't make sense at all.
    Yes it does.

    Quoting you:

    Surely if you are editing someone elses code, you have to know what the variable starts with anyway - if you are typing the variable name out, you already know if it's public or private because you have to know whether to start with a _ or not.
    Example: You're editing someone else's code and see $this->config. Their code is consistent with Zend coding standard--therefore you know this variable is public.

    Which is exactly what I said:

    You know if it's public or private *because* you're using the underscore naming convention. If you aren't then you have no idea without looking at the definition.
    If that person was not using a coding standard and you see $this->config then you have no idea what the visibility of that variable is.

    Quote Originally Posted by logic_earth View Post
    ^^^ In all three examples you have to look up all the vars anyways to know exactly what they are for, making the point moot. Why would one blindly use a variable one sees in the code?
    Since when do you have to read the documentation for every method and every property in a class?

    Good code is self documenting and a decent portion of the time you know what the code does without reading the documentation.

    If you're in a method called setName($name) do you really have to read the @param phpdoc to figure out what $name does?

    If you see the method _htmlToAscii() used, do you really have to go read the entire phpdoc block to figure out what the function does?

    No, you don't. And even seeing just once usage of _htmlToAscii() you know it's private/protected and is not available outside of the class.
    Brad Hanson, Web Applications & Scalability Specialist
    ► Is your website outgrowing its current hosting solution?
    ► PM me for a FREE scalability consult!
    ► USA Based: Available by Phone, Skype, AIM, and E-mail.

  24. #24
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by bhanson View Post
    Since when do you have to read the documentation for every method and every property in a class?
    I meant look up at the declaration of the class variable in the source code.
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  25. #25
    SitePoint Guru
    Join Date
    Jan 2005
    Location
    heaven
    Posts
    953
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Stormrider View Post
    NuSphere PHPEd. I used to use Zend, but it ran very slowly on my PC. PHPEd seems to have everything that I liked about Zend, plus more, and runs a lot faster as well (it isn't built in Java...).
    I don't subscribe to IDEs or WYSISWYG editors. I use, vim/vi or a plain text editor. Java offends me and I try to avoid using anything made in java as much as is permissible...

    Quote Originally Posted by Stormrider View Post
    Surely if you are editing someone elses code, you have to know what the variable starts with anyway - if you are typing the variable name out, you already know if it's public or private because you have to know whether to start with a _ or not. If you don't know which it is, and DON'T use underscores, you can start typing the name and autocomplete will do the rest, and tell you what type it is
    So, would you like all developers to abandon good programming habits simple because some tool can "autocomplete" and "and tell you what type it is"? You never program for yourself today. You program for yourself and everyone else tomorrow. Who's to say the day won't come when you or a co-worker, if you have one, won't have an IDE? How will you or that person function then?

    Quote Originally Posted by logic_earth View Post
    I meant look up at the declaration of the class variable in the source code.
    We don't all have the benefit of scrolling. My scrolling is Page Up, Page Down, Arrow Up and Arrow Down. Scrolling becomes laborious with a file that contains a 1000+ lines, of code or other wise.
    Creativity knows no other restraint than the
    confines of a small mind.
    - Me
    Geekly Humor
    Oh baby! Check out the design patterns on that framework!


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
  •