Problems with parent / inheritance in PHP

Question

EDIT: I got this working now. I have updated the code:

I've looked at a few examples in here, and they all look logical. But I can't get it to work. I have a class that extends my Data Access Layer (DAL). I would like to call parent class to retrieve DB results. What am I doig wrong?

DAL class

class DAL { 
  protected $username;     // the username for db connect 
  protected $pwd;          // the pwd to use when connecting 
  protected $host;         // the host to which one connects 
  protected $dbname;       // the db to select 
  public $conn;          // reference to the db link resource 
  public $db;            // result of db_select 
  public $query_result;  // the stored result of the last query you ran 

  public function __construct() 
  { 
    $this->username     = ""; 
    $this->pwd          = ""; 
    $this->host         = ""; 
    $this->dbname       = ""; 
  } 

  public function connect() 
  { 
    /* connects to DB here */
  }  

  private function query($sql) 
  {
    /* Executes the query here and stores the result in $this->query_result */
  }

  public function getAllCountries()
  {
    $sql =" 
      SELECT id, name
      FROM country";

    //Process query
    $this->query($sql);

    if($this->query_result)
      return $this->query_result;      
  }
}

And this is my other class

class myOtherClass extends DAL{

  public function __construct() {
   parent::__construct();
   parent::connect();
  }

  public function getCountryListBox()
  {
    $result = parent::getAllCountries();

    if($result)
    {
      $selectbox = "<select id='countryListBox' name='countryListBox'>";

      while ($row = mysql_fetch_array($result))
      {
        $selectbox .= "<option value='".($row['id'])."'>".($row['name'])."</option>";
      }
      $selectbox .= "</select>";
    }
    else
      $selectbox = "Countries could not be retrievd from database.";

    return  $selectbox;    
  }  
}

This is the code in my template:

$sl = new myOtherClass();

echo '<form id="label_data_form">';
  $sl->getCountryListBox(); 
echo '</form>';

Answer 1

Steven, let me draw your attention to other things, that are incorrect in your sample despite being syntactically correct. The major issue here is responsibility separation: Data Access Layer should only act as a general purpose data retrieving/storing utility class. Any additional logic should be moved outside this class.

1. Database connection handling should not be part of DAL. Ideal solution is to pass the db object in constructor, so the DAL operates on the connection configured somewhere else. This is called Dependency Injection and is generally regarded a good thing.

2. Method getAllCountries is way too specific for general purpose library. Should be replaced with getAll returning all records from current table. Table name should be passed in constructor or defined in subclass, so that every table has its corresponding DAL object.

3. *Method getCountryListBox generates some HTML output, which is not part of responsibility of DAL library. DAL should only return raw data.

It is worth keeping things separated, so you can reuse them in the future. Adding too many problem specific extension blurs the responsibility of a class. Main objectives of a class should be very narrow-minded, so classes can specialise in doing different things. Cooperation between several highly specialised classes should be the way of delivering complex functionality.

Answer 2

Update

You should add error handling logic inside query() and getAllCountries().

this->getAllCountries(); should be fine, provided that it was a type and you actually meant $this, ie with a dollar.

I'd also like to point out missing logic in getAllCountries() for cases when the query() method is returning nothing and thus $this->query_result holds nothing. getAllCountries() should return something like an empty array, because there may be cases when there's not country in the DB. You want to be able to handle that.

Answer 3

The difference between these:

// $result = parent::getAllCountries();  // Like this
$result = this->getAllCountries();       // Or like this?

.. is probably best explained here:

class SuperFoo {
    function fubar () {
        echo "superfoo!";
    }

    function fuz () {
        echo "superfuz!";
    }
}

class SubBar extends SuperFoo {
    function fubar() {
        echo "subbar!";
    }

    function demonstrate() {
        $this->fubar();    // "subbar!"
        parent::fubar();   // "superfoo!"

        $this->fuz();      // "superfuz!"
        parent::fuz();     // "superfuz!"
    }
}

$s = new SubBar();
$s->demonstrate();

(okay, maybe not best explained..)

Unless you particularly want the behaviour defined in the parent class, I'd always use $this->... since then you have the option of altering the behaviour if needed.

---

The MySQL error seems to be caused by a problem with your SQL - the class inheritance looks to be working fine.

Answer 4

Are you sure that storelocator is an instance of MyOtherClass? Because the method you are calling, getCountryListBox is defined in MyOtherClass, and the error complains about an undefined methods getCountryListBox...

<?php $sl = new storelocator(); ?>
<form id="label_data_form">  
<?php $sl->getCountryListBox();  ?>

Answer 5

Answer to you edit?

Another typo:

but I guess you have this correct (-> in stead of -) in your code.

Are you sure the 'myOtherClass' is the 'storelocator' class?

Answer 6

for the part you want to fetch list of all countries:

// $result = parent::getAllCountries();  // Like this
$result = $this->getAllCountries();       // Or like this?

the second line is correct. getAllCountries() is a method of your object, because it is inherited from your parent class. so you do not need to call the parent method. one place that you would want to call class parent's methods are in methods where you are overwriting the method.

anyways I suggest you define your properties in your DAL class as 'protected' instead of 'private'. because when you define them 'private', they can not be inherited.

so in your DAL class:

protected $username;     // the username for db connect 
protected $pwd;          // the pwd to use when connecting 
protected $host;         // the host to which one connects 
protected $dbname;       // the db to select

you should define those properties as private, only if you do not want them to be inherited. I think your new object needs these info to connect to database.

Answer 7

Generally speaking, you would use

$result = $this->getAllCountries();

(Note the **$**this rather than this!)

When calling getAllCountries from anywhere other than the getAllCountries method itself. This will ensure that if you were to override it, the overriden method would be called.

Inside the method, if you wanted to call the base class version, you would need to disambiguate the call with parent::

parent::getAllCountries();

Note you can use this form of method call within any method, but I would argue that it would be bad practice to do this by default, since it denies you the opportunity to override that method later on.

Answer 8

if your class 'myOtherClass' does not implement the 'getAllCountries()' method, the following should work:

$result = $this->getAllCountries();    

do not forget the $-sign before 'this'!