HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PHP class design with methods requiring database access

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withphpdesignmethodsdatabaseclassaccessrequiring

Problem

I've been a procedural PHP programmer for a long time but I'm getting to learn OOP and would like some advice. I'm designing a class which currently is composed mainly of simple getters/setters, but I'm not quite sure if I'm designing my class in the best way possible.

The getters require accessing a database to pull the requested information, so I'm checking if a property has been set first before running the query which seems like a good idea to me but I'm not sure.

I think I could make use of DI by type hinting the $connect parameter and using a DAL but this is not a public facing API and we're never going to use a different database engine so I'm not sure it's worth the trouble. I'm using a proprietary database engine which has a bug preventing me using PDO and parametrised queries. I'm also not sure if I should be try/catching/throwing exceptions inside class methods.

Here is a sample of the code below, there are more methods/properties but they all follow the same basic structure. I would very much appreciate any and all commentary.

```
class Patient {
// Database connector
private $connect;
protected $_connection;

public $dmg_ID;
public $LNK_ID;
public $dailyLNK_ID;

public function __construct($connect, $lnkid, $dlkid = NULL) {
$this->_connection = $connect;
unset($connect);

// Patient requires a LNK_ID but not always a daily link ID
$this->LNK_ID = $lnkid;

if (!is_null($dlkid) && is_numeric($dlkid)) {
$this->dailyLNK_ID = $dlkid;
}
}

public function getDmgID() {

if ($this->dmg_ID) return $this->dmg_ID;

$sql = "SELECT lnk_dmgID FROM LINK WHERE lnk_ID=".$this->LNK_ID;
try {
$result = odbc_exec($this->_connection,$sql);
if($result) {
$dmgID = odbc_fetch_array($result);
}
else {
throw new RuntimeException("Failed to connect.");
}
}
catch (RuntimeException $e) {
prin

Solution

OOP is definitely the way to go. The best advise I can offer is to read the book "Clean Code" by Uncle Bob. The code there is in Java but all principles apply equally to any other language.

I would definitely abstract the DB interface so I can use for any other database, even if you don't plan it in the next future. I have posted small data store architecture code that may help you.

From a look at your code, I am a bit confused by the naming connect and connection and the latter stores $connect in the constructor. Maybe give a more descriptive names (one of the top recommendations in the mentioned book).

-
Why are you unsetting $connect?

-
The names such as $LNK_ID are cryptic for reader.

-
I would reserve a separate class dealing with the database via thin interface (see my post) and let other classes only talk to that class instead of DB directly.

Context

StackExchange Code Review Q#41067, answer score: 2

Revisions (0)

No revisions yet.