How to avoid a construct being called twice

Question

Currently trying to learn the "right" way to build a site with OOP and the MVC model, and obviously running into some stumbling blocks since I've been doing mostly procedural for years.

(I'm also using PHP 7 for the first time since I've been stuck with PHP 5 for years and years, so also trying to code the "right" way using mysqli_ instead of the old mysql_. Learning prepared statements for the first time also instead of just escaping all my variables, but that's a whole different discussion)

Right now I have a few different classes:

config.class.php -> Config -> set up the db connection and other site settings
leads.class.php -> Leads extends Config -> get lead info from the db upon request (when someone hits the site)
questions.class.php -> Questions extends Config -> get question info (it's a survey site)
leadsview.class.php -> -> LeadsView extends Leads -> display lead info if needs be
questionsview.class.php -> QuestionsView extends Questions -> display question info on the page after it's pulled from the db

So, let's say on my index.php I need to instantiate both QuestionsView and LeadsView so I can display questions and lead info on the page, respectively. So I do this:

session_start();
include_once 'lib/autoload.php'; // class autoloader
$l = new LeadsView();
$q = new QuestionsView();

Totally works fine so far. BUT - in my Config class I not only have a database connection, but a few site configuration settings inside a constructor that need to be set when someone first hits the page. Something like this so far:

class Config {
  // set up db connection
  private $db_host;
  private $db_user;
  private $db_password;
  private $db_name;
  protected $conn;
  protected $flow_id;
  protected $domain;

  public function __construct() {
    $this->flow_id = $_REQUEST['flow'];
    $this->domain = $_SERVER['HTTP_HOST'];
    $this->connect(); // open the db connection

    print "hey<br>";
  }

  protected function connect($db = "default") {
    switch ($db) {
      // allow for different db connections in the future
      case "default":
        $this->db_host = "XXXX";
        $this->db_user = "XXXX";
        $this->db_password = "XXXX";
        $this->db_name = "XXXX";
      break;
    }

    $this->conn = new mysqli($this->db_host, $this->db_user, $this->db_password, $this->db_name);
    $this->conn->set_charset("utf8mb4");

    if ($this->conn->connect_error) {
      die("Error connecting to db: " . $this->conn->connect_error);
    }
  }
}

I threw in that print "hey<br>"; just to make sure that constructor was being run. But now it's being run twice: once from my $l = new LeadsView(); and once from $q = new QuestionsView(); since they both extend Config. I don't need or want to run that constructor twice, for obvious reasons.

So my question is: what's the proper way to set up a configuration class in a scenario like this? Should I create a Dbh class separate from Config, and just have all my questions, leads, etc. classes extend Dbh instead so they don't all run the Config methods as well? Is there some other obvious way to do this that I'm completely missing?

Answer 1

Your classes should never extend from Config or Database class or anything like that. This is not how polymorphism works.

In fact, your Config class isn't very useful. To make use of such class you need methods that actually help you perform prepared statements. The connect() method is not needed and you definitely do not need all the properties.

If we were going to fix this class then we could do something like this:

class Database {
  protected $conn;
  protected $flow_id;
  protected $domain;

  public function __construct(string $db) {
    $this->flow_id = $_REQUEST['flow'];
    $this->domain = $_SERVER['HTTP_HOST'];

    mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
    // Load the variable from the config file
    $this->conn = new mysqli($db_host, $db_user, $db_password, $db_name);
    $this->conn->set_charset("utf8mb4");

  }
    //
    // The rest of your database abstraction methods. Some methods that help you write parameterized queries more easily
    // Without other methods this class would be useless!
}

The name of the class doesn't make sense. It is not a config class. It is an abstraction around mysqli. The config details should be stored in a separate file.

When you need to use this class in another class, simply require it as a parameter.

$db = new Database();
$l = new LeadsView($db);

This is called dependency injection. You should use this pattern in your code when working with OOP as it will make a lot of interactions between your classes much simpler. You can also implement IoC container. Nette has a good tutorial, but you can also use their implementation.

I would strongly recommend to don't reinvent the wheel. There are already good libraries that provide abstraction around PDO. Yes, PDO not mysqli. Do not waste your time with mysqli unless you have an extremely good reason to use mysqli. Use PDO with an abstraction library such as EasyDB

Answer 2

You are using inheritance. You probably want to use composition instead. Concretely this means providing the Config to LeadsView or Questionsview.

$config = new Config();
$leadsview = new LeadsView( $config );
  
class Config {
  __construct() {
    // do whatever
  }
}

class LeadsView {
  private $config;

  __construct( $config ) {
    $this->config = $config;
  }

  // use this->config somewhere in Leadsview to get whatever you need
}

To prevent the usage of multiple instances of Config you can use a single reference (this requires your discipline to enforce).

Otherwise you can create config as a singleton, which guarantees only one version of Config exists. If you want to know more about Singletons you can look it up, it's a very well know pattern.