SitePoint Sponsor

User Tag List

Results 1 to 8 of 8
  1. #1
    SitePoint Zealot Steveiwonder's Avatar
    Join Date
    Nov 2008
    Posts
    151
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    My first real attempt with OOP.

    Morning peeps,

    I was wondering if people could advise of better ways to do what i've attempted to.

    Its just a small class to carry out a select query, as i'm learning i guess i need critism for me to learn from my mistakes so fire away. Ignore how i have displayed the data as thats not what im intrested in, that was just to show it worked.
    Beleive me, its nothing fancy!

    Class:
    Code PHP:
    <?php
    class select {
      var $query; // will store the query string only.
      var $result; // will store the results after the execute function is called.
     
      function __construct($columns, $table, $clause){
        // Create the query and store in var "query"
        $query = "SELECT ";
        $query .= implode(", ", $columns); // separate the colums.
        $query .= " FROM ".$table;
        $query .= $clause;
        $this->query = $query;
      }
     
      function execute (){
        //execute the query and store the results into the var "results";
        $this->result = mysql_query($this->query);
     
      }
    }
    ?>

    view page:

    Code PHP:
    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml">
    <head>
    <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
    <title>OOP in PHP</title>
    <?php include('class_lib.php'); ?>
    <?php include('conn.php'); ?>
    </head>
    <body>
    <?php
    //setup query parameters
    $columns = array("username","email"); // select these columns.
    $table = "users";// tables to select from
    $clause = " WHERE user_id !='1'"; //class query, options.
     
    $q_get_usrname_email = new select($columns, $table, $clause); // create a new object for select.
    $q_get_usrname_email->execute(); // execute the query.
    //output the query.
     
    while ($row = mysql_fetch_assoc($q_get_usrname_email->result)){
      echo $row['username']." ".$row['email']."<br>";
     
    }
     
    ?>
     
     </body>
    </html>

    Thanks for any pointers in advance.

    Steve
    Follow the dream, don't chase the competition.

  2. #2
    PHP Guru lampcms.com's Avatar
    Join Date
    Jan 2009
    Posts
    921
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Why use the php4 style?

    I would recommend using 'protected' keyword instead of 'var'
    and definetely dont make the $result public. You should either make it protected and have accessor method OR just have your execute method return the result.

  3. #3
    SitePoint Zealot
    Join Date
    Dec 2008
    Posts
    120
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    that is simple class but what about JOINs UNIOINs and in general for complex queries.

    My opinion is that this class is not so usefull. Try to make business entities and capsulate business logix in classes.

  4. #4
    SitePoint Addict
    Join Date
    Apr 2009
    Posts
    248
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I'm with Kuzmanin. I think you're trying to replicate functionality that already exists as a baseline in PHP.

  5. #5
    SitePoint Zealot Steveiwonder's Avatar
    Join Date
    Nov 2008
    Posts
    151
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    oh i know i know, i wont be using this code this is just me making up more or less, pointless and useless classes just as a challenge for me. I just want to know if my coding techniques are terrible and need improving etc.

    I'll work on something else thats more relevant i guess.

    Thanks anyways.
    Follow the dream, don't chase the competition.

  6. #6
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,151
    Mentioned
    16 Post(s)
    Tagged
    3 Thread(s)
    Well properties should always be protected or private. Properties should only be accessible to the outside world indirectly through getter and setters. Furthermore, if a method only needs to be accessed from within the scope of the object then you should consider having those be protected or private also. Limiting the methods which are accessible outside the scope of the object leads to a simplified public interface. The simpler the public interface the easier the class is to use and understand months or years down the line.

  7. #7
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    It's better practice to keep access to variables through methods (functions) instead of direct access.

    Maybe you could try something like....
    PHP Code:
    class Select{
        protected 
    $Table$Columns = array(), $Clauses = array(), $Result false$LastQuery '';
        public function 
    __Construct($Table, array $Columns null, array $Clauses null){
            
    $this->setTable($Table);
            
    $this->addClause('1 = 1');
            if(
    $Columns != null){
                
    $this->addColumns($Columns);
            }
            if(
    $Clause != null){
                
    $this->setClause($Clause);
            }
        }
        public function 
    setTable($Table){
            
    $this->Table = (string)$Table;
        }
        public function 
    addClauses(array $Clause){
            
    $this->Clauses array_merge($this->Clause$Clause);
        }
        public function 
    addColumn($Clause){
            
    $this->Clauses[] = (string)$Clause;
        }
        public function 
    addColumns(array $Columns){
            
    $this->Columns array_merge($this->Columns$Columns);
        }
        public function 
    addColumn($Column){
            
    $this->Columns[] = $Column;
        }
        public function 
    getResult(){
            return 
    $this->Result;
        }
        public function 
    getLastQuery(){
            return 
    $this->LastQuery;
        }
        public function 
    fetchArray(){
            if(
    $this->Result !== false){
                return 
    MySQL_Fetch_Array($this->Result);
            }else{
                return 
    false;
            }
        }
        public function 
    execute(){
            
    $Query SPrintF('SELECT %s FROM %s WHERE %s'count($Columns) > implode(', '$Columns) : '*'$this->Tableimplode(' AND '$this->Clauses));
            
    $Result MySQL_Query($Query);
            return 
    $this->getResult();
        }

    I've just written it out here so it's untested, but it's implementation would be:
    PHP Code:
    <?php
    $Select 
    = new Select('Food', array('Name'));
    $Select->execute();
    echo 
    '<h3>All Food:</h3>';
    echo 
    '<ul>';
    while(
    $row $Select->fetchArray()){
        
    printf('<li>%s</li>'$row['Name']);
    }
    echo 
    '</ul>';

    $Select->AddClause("Type = 'Fruit'");
    $Select->execute();
    echo 
    '<h3>Just Fruit:</h3>';
    echo 
    '<ul>';
    while(
    $row $Select->fetchArray()){
        
    printf('<li>%s</li>'$row['Name']);
    }
    echo 
    '</ul>';

    $Select->AddClause("Name LIKE 'a%'");
    $Select->execute();
    echo 
    '<h3>Just Fruit Beginning with A:</h3>';
     echo 
    '<ul>';
     while(
    $row $Select->fetchArray()){
         
    printf('<li>%s</li>'$row['Name']);
     }
     echo 
    '</ul>';
    A query class doesn't have many uses, but if you were to use one and base it on MySQL (PDO would be a much better option) then the above should be a good example.
    Jake Arkinstall
    "Sometimes you don't need to reinvent the wheel;
    Sometimes its enough to make that wheel more rounded"-Molona

  8. #8
    Twitter: @AnthonySterling silver trophy AnthonySterling's Avatar
    Join Date
    Apr 2008
    Location
    North-East, UK.
    Posts
    6,111
    Mentioned
    3 Post(s)
    Tagged
    0 Thread(s)
    You should also try to separate the dependency on MySQL - as the Select object should only be concerned with building the right SQL string.

    Pass the Select object to an executor of sorts, the executor should accept a Select object and return an array of results.

    This would further allow you to utilise many other storage mediums without modifying the Select object's code.
    @AnthonySterling: I'm a PHP developer, a consultant for oopnorth.com and the organiser of @phpne, a PHP User Group covering the North-East of England.


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
  •