2
\$\begingroup\$

Is it a bad idea for a model class hold the db handler as a property? Example code are as below. I think the second is better, but the first is not so bad in my opinion.

First:

class ActiveModel {
    protected $db;

    public function __construct() {
        $this->db = DbFactory::get('Foo');
    }

    public function save() {
        //use $this->db to do the job;
        ....
    }
}

Second:

class ActiveModel {

    public function save($db = null) {
        if ($db === null) {
            $db = DbFactory::get('Foo');
        }
        //then use $db to do the job;
        ....
    }
}
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

It is fine and very convenient to keep the database handler as a property. However, you should pass it in the constructor via dependency injection, rather than call it's factory method inside the class. This can ease unit testing when you need to mock a database connection object.

public function __construct($db) {
    $this->db = $db;
}
\$\endgroup\$
2
  • \$\begingroup\$ Yes, you are right. But when dependency injection is not used, I need pass it to constructor every time. When by calling the factory method, I just need change the setting of DbFactory to let it give the mock db connection object. \$\endgroup\$
    – xdazz
    Sep 15, 2011 at 2:38
  • 1
    \$\begingroup\$ Also, when doing automated testing, if DbFactory::get('Foo') becomes expensive your test suite will run much slower. \$\endgroup\$ Sep 15, 2011 at 16:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.