SitePoint Sponsor

User Tag List

Results 1 to 11 of 11
  1. #1
    SitePoint Member
    Join Date
    Mar 2005
    Posts
    7
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Sloppy class please help

    Hi, im new to php and im trying to learn by building a text based car game. The problem im having is i designed a class to list all the cars a player has or can purchase new or is in the scrapyard. The problem is i have really badly designed it and i found a huge flaw.

    This is the class
    PHP Code:
    class CarList
    {
         
         
    //class attributes
         
    var $title;
         var 
    $description;
         var 
    $sql;
         var 
    $nores;

         
         var    
    $i '0';
        var 
    $n '4'//starting row cars
        
    var $s '4'//cars per row
         
        
    public function __construct($title$description$sql$nores 'No Cars to Display!'){
            
    $this->title $title;
            
    $this->sql $sql;
            
    $this->description $description;
            
    $this->nores $nores;
            
    $this->DisplayCars();
        }
        
        
         protected function 
    DisplayCars()
         {
            echo 
    '<h1>'.$this->title."</h1>\n".$this->description."\n";
            echo 
    "<table class=\"products\">\n";
            echo 
    "<tr>\n";
                        
            require(
    'inc/db/dbconnect.inc.php');
            
            @
    $query $db_conn->query($this->sql)
                or die(
    'Failed to connect to database and retrieve your Cars! Please report this problem.');
                
            while (
    $cars $query->fetch_array())
            {        
        
                
    $tbl $this->DrawTable($cars$this->DisplayImage($cars$this->ImagesExists($cars)));
            
                echo 
    $tbl;
            
                
    $this->i++;

            }
            
            if (
    $this->== '0')
            {
                echo 
    '<td><b>'.$this->nores."</b></td>\n";
            }
            
                    echo 
    '</tr></table>';
            
        }
        
        
        protected function 
    ImagesExists($cars)
        {
            if (
    file_exists('images/cars/'.$cars['imgsrc']) && !empty($cars['imgsrc']))
             {
                  return 
    true;
            }else{
                 return 
    false;
            }
        }
        
        
        protected function 
    DisplayImage($cars$exists true$sclass 'gsel'$nclass 'gnsel')
        {
            if (
    $exists == true)
             {
                  if ( 
    $cars['selected'] == '1')
                  {
                    
    $img '<img src="images/cars/'.$cars['imgsrc'].'" '.ImageResize('images/cars/'.$cars['imgsrc'], 100).' alt="'.$cars['car'].'" class="'.$sclass.'" />';
                }else{
                    
    $img '<img src="images/cars/'.$cars['imgsrc'].'" '.ImageResize('images/cars/'.$cars['imgsrc'], 100).' alt="'.$cars['car'].'" class="'.$nclass.'" />';
                }
            }else{
             
                if ( 
    $cars['selected'] == '1')
                  {
                    
    $img '<img src="images/cars/none.gif" alt="'.$cars['car'].'" class="'.$sclass.'" />';
                }else{
                    
    $img '<img src="images/cars/none.gif" alt="'.$cars['car'].'" class="'.$nclass.'" />';
                }
            }
            
            return 
    $img;
        }
                
        
        protected function 
    DrawTable($cars$img)
        {
            if (
    $this->== $this->n)
             {
                
    $tbl "\n</tr>\n<tr>\n<td><a href=\"index.php?p=cardet&amp;item=".$cars['carid'].'&amp;tid='.$_SESSION['tid'].'">'.$img.'</a><br />'.$cars['car'].' - $'.$cars['price'].'.</td>';
                
    $this->+= $this->s;
                
            }else{
             
                
    $tbl "\n<td><a href=\"index.php?p=cardet&amp;item=".$cars['carid'].'&amp;tid='.$_SESSION['tid'].'">'.$img.'</a><br />'.$cars['car'].' - $'.$cars['price'].'.</td>';
            }
            
            return 
    $tbl;
            
        }


    It is accessed on a page called garage, which is all the cars a player owns, like this. This will also work for cars in the player scrapyard.
    PHP Code:
              $sql "SELECT
                    cars.carid,
                    cars.car,
                    cars.description,
                    cars.`type`,
                    cars.imgsrc,
                    garage.price,
                    garage.bhp,
                    garage.speed,
                    garage.selected
                FROM
                    garage
                    Inner Join cars ON garage.carid = cars.carid
                WHERE
                    garage.userid = '
    {$_SESSION['uid']}'";
                    
        
    $garagelist = new CarList('Welcome to Your Grage''Below are your cars.'$sql'You need to Buy a Car!'); 

    The huge problems is that, because i am using a ref table to hold the players cars and a car table to hold the cars, i cannot list all the cars a player does not have.

  2. #2
    SitePoint Addict
    Join Date
    Feb 2007
    Posts
    251
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I only had enough time to take a cursory look over the source, so forgive me if this is a silly question, but is there anything preventing you from using a left join? For instance --

    SELECT cars.car, NOT garage.userid IS NULL AS has_car, ... FROM cars LEFT JOIN garage ON cars.carid = garage.carid AND garage.userid = $userid

  3. #3
    SitePoint Member
    Join Date
    Mar 2005
    Posts
    7
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I may be wrong but surley that would still display the cars a player has.

    Thanks anyway
    George

  4. #4
    Spirit Coder allspiritseve's Avatar
    Join Date
    Dec 2002
    Location
    Ann Arbor, MI (USA)
    Posts
    648
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I'm not completely familiar with the syntax cuberoot used, but: a left join takes two tables, and joins them. Because it's a left join, and not an inner join, the rows in the left table (the first in the query) have to exist. The rows in the right table do not. So, if your left table is all cars, and your right table is player cars, and you join them using a common field (typically a car_id, or something similar) you will end up with some rows with player car information, and some cars with NULL as their values. These rows correspond to cars in the cars table that are not in the players table, which is what you want.

    HTML Code:
    SELECT * FROM cars LEFT JOIN player_cars ON (cars.car_id = player_cars.car_id) WHERE player_cars.car_id IS NULL

  5. #5
    SitePoint Member
    Join Date
    Mar 2005
    Posts
    7
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Not quite, if i ran that sql it would list all the cars that are not owned by any player. Because more than one player can own the same car it wouldnt work. cuberoots code would work if i add an if statment to the loop like so
    PHP Code:
            while ($cars $query->fetch_array())
            {        
                if (
    $cars['has_car'] != true)
                {
                    
    $tbl $this->DrawTable($cars$this->DisplayImage($cars$this->ImagesExists($cars)));
            
                    echo 
    $tbl;
                    
                    
    $this->i++;
                }
            } 
    The problem with this, for me anyway, is what already seems like sloppy code gets worse as i would need to check the var is set. The way i see it is this is a very simple idea but when coded i can get it as simple as it should be in my mind.

    Thanks

  6. #6
    Spirit Coder allspiritseve's Avatar
    Join Date
    Dec 2002
    Location
    Ann Arbor, MI (USA)
    Posts
    648
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Geo200k View Post
    Not quite, if i ran that sql it would list all the cars that are not owned by any player.
    Ah, I thought the player_cars would just have a single player's cars in it. In that case:

    HTML Code:
    SELECT * FROM cars LEFT JOIN player_cars ON (cars.car_id = player_cars.car_id AND player_cars.user_id = :user_id) WHERE player_cars.car_id IS NULL
    (Note: I use PDO, so you could substitute :user_id with however you'd place a variable in a query)

  7. #7
    SitePoint Member
    Join Date
    Mar 2005
    Posts
    7
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Yep that does it. Any advise on my code as im very new to PHP.

    Thanks alot
    George

  8. #8
    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)
    Are you new too OOP or PHP? What other languages have you learned then?

  9. #9
    SitePoint Addict
    Join Date
    Feb 2007
    Posts
    251
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I'd break the database specific stuff out of the class, since it really hurts reusability. I'd also separate the structure of the view from the rendering process. Something like this --

    PHP Code:
    class View {
        function 
    accept($visitor) {
            
    $visitor->visit($this);
        }
    }

    class 
    CarView extends View {
        protected 
    $cars;
        function 
    __construct($cars=null) {
            if (
    $cars) {
                foreach (
    $cars as $car) {
                    
    $this->addCar($car);
                }
            }
        }
        function 
    addCar(Car $car) {
            
    $this->cars[] = $car;
        }
        function 
    getCars() {
            return 
    $this->cars;
        }
    }

    abstract class 
    HtmlTableRenderer {
        protected 
    $tplTable;
        protected 
    $tplRow;
        protected 
    $html;
        function 
    __construct($tplTable '<table>%s</table>',
            
    $tplRow '<tr><td>%s</td><td>%s</td></tr>') {
            
    $this->tplTable $tplTable;
            
    $this->tplRow $tplRow;
        }
        abstract function 
    visit($view);
        abstract function 
    render();
    }

    class 
    CarViewTableRenderer extends HtmlTableRenderer {
        function 
    visit($view) {
            if (
    $cars $view->getCars()) {
                foreach (
    $cars as $car) {
                    
    $this->html .= sprintf($this->tplRow,
                        
    $car->getName(), $car->getDescription());
                }
                
    $this->html sprintf($this->tplTable$this->html);
            }
        }
        function 
    render() {
            return 
    $this->html;
        }
    }

    class 
    Car {
        private 
    $name;
        private 
    $description;
        function 
    __construct($name$description) {
            
    $this->name $name;
            
    $this->description $description;
        }
        function 
    getName() {
            return 
    $this->name;
        }
        function 
    getDescription() {
            return 
    $this->description;
        }
    }

    $cars = array();
    $cars[] = new Car('Pinto''The finest automobile ever made');
    $cars[] = new Car('Yugo''A huge commercial success');

    /*
    $cars could obviously be an iterator on a database query result,
    or a collection of nodes parsed from a web service, etc.
    */

    $carList = new CarView($cars);
    $carList->accept($renderer = new CarViewTableRenderer());
    echo 
    $renderer->render(); 
    This article offers further insight into this approach --
    http://devzone.zend.com/node/view/id/9

  10. #10
    SitePoint Member
    Join Date
    Mar 2005
    Posts
    7
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    cups - im new to php and oop, i have done a little in java but never fully grasped it and therefore my oop knowledge is very limited. I did a lot in vb as a student so am familiar with procedural code.

    cuberoot - i see makes much more sence to do it like that.

    Could anyone recomend any books on oop and design pattens.

    Thanks George

  11. #11
    Foozle Reducer ServerStorm's Avatar
    Join Date
    Feb 2005
    Location
    Burlington, Canada
    Posts
    2,699
    Mentioned
    89 Post(s)
    Tagged
    6 Thread(s)
    Hi Geo200k,

    Could anyone recomend any books on oop and design pattens.
    Before you dig yourself into the fowler books (i.e Patterns for Enterprise Architects) you should check out 'Guide to PHP Design Patterns' by Jason E. Sweat. This book is centered around patterns but will also help you understand a good way to code and test your code.

    It's isbn# = 0-9735898-2-5 and it is published by php|architect so you will likely be able to order it right off their site; however I have not verified this.

    Regards,
    ServerStorm
    ictus==""


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
  •