The simple things:
A quick canter through the doubts/questions you're having.
The code doesn't seem to have any bugs
That's not entirely true. The fact that your code uses the mysql
extension will produce E_DEPRECATED
notices if you attempt to run this code on PHP 5.5 and up. The extension is deprecated, as the red warning box on the doc pages will tell you.
When developing/testing new code, always turn display_errors
on, and set the error_reporting
to E_STRICT|E_ALL
. Notices are a sure-fire way to slow your code down (seeing as the notices are being produced anyway, and only hushed up at the last moment).
mysql
also doesn't support prepared statements, so you might end up with injection vulnerabilities. This means you'd have to spend a lot more time thoroughly testing your code, and wasting a lot more hours on security testing that you normally would.
whether using object orientation will be better (speed etc) than the functional way
First off, allow me to be pedantic and say that PHP does really do functional programming. PHP functions are not first class objects: they can't be returned by functions, or be passed around as arguments, or assigned to variables. Sure, PHP has anonymous functions, which are actually objects (get_class(function(){});
will tell you that anonymous functions are instances of the Closure
class. Functional languages are Lisp, Scheme, Haskell, JavaScript and so on. PHP originally was a procedural language, that now supports OOP, too.
There are some constructs that allow you to adopt a more functional style of programming, but that's more syntactic sugar than it is actual functional programming.
Having said that, your question is an easy one to answer: OO will almost never be faster. OOP wasn't introduced to increase performance, or at least not runtime performance. Its main purpose was to increase code re-usability, cut down on development (mainly maintenance) time, create cleaner, more intuitive and easy to learn API's and all that. In exchange, a little overhead is a price worth paying (in most cases).
If all you're really after, you'd still be best served using regular procedural code. To increase performance even more, you'd probably want to keep the number of files to a minimum (to reduce disk I/O), and you might want to replace all loops like this:
foreach ($array as $k => $v) {
//do stuff
}
With the faster:
foreach ($array as &$v) {
//do stuff
}
It's faster (or at least, last I checked it was), but there are some major drawbacks you'll have to take into consideration: more error-prone code, hellish to debug, harder to read, understand and therefore harder to maintain,... Basically, it's just not worth it.
Bottom line is: don't worry about speed too much. If you're using PHP 5.5 and up, or use APC to cache the OP-Codes, you won't be able to tell the difference. Micro optimization is the root of all evil and all that.
OO will make a huge difference when your codebase starts to grow in size. You'll be happy with things like namespaces and classes that bundle functionality nicely without your having to wonder which files you need to require
, and whether or not you've require
'd it already
The code itself
Now moving on to the actual code
Standards matter
It's something I can bang on about for quite some time, but it boils down to this: Code should be easy to read for anyone familiar with the language. If we all write code the way we like it, the coding style can become a hindrance. Just think of it like this: you can read (obvioulsy), but some people's handwriting can be quite hard do decipher. The same principles apply to code, and to get around the handwriting problem, the PHP community, for the largest part, has adopted/accepted the PHP-FIG standards. You should to.
Looking at your code, this means that:
<?php
class getResources{
Should become:
<?php //no leading whitespace
//classes start with an UpperCase, get is for methods
class Resources
{//<== goes on the next line
Other things you need to fix include:
function getContent($rowId, $tableName){
//method body
}
//...
?>
Fixes needed:
- Missing access modifier (
public
by default, but should be added explicitly)
- methods' opening
{
goes on a new line
- Files containing nothing but PHP code should not have a closing
?>
tag
What's the point?
Now seeing as you're going to have to rewrite this class anyway, to use one of the replacement extensions for mysql
, you might want to look at this review. You're essentially writing a wrapper object. Given that the new extensions (in particular PDO
) has a very clean API right out of the box, I'd strongly suggest you didn't bother to try and wrap it up in a class of your own. At least not in the way you're doing now. You'd have to create an actual abstraction layer, which has been done before (Doctrine's DBAL, for example).
Again, this is something I could drone on about for ages, like I did in the review I linked to, but the gist of it really is this: Attempting to wrap the new DB extensions into a single class is going to end up in tears. That's a promise. What you need to realize about this matter is that code that interacts with databases actually consists of several distinct resources:
- A DB connection (a
PDO
instance, or a mysqli
instance)
- A handle to a prepared statemend (a
PDOStatement
instance, or a mysqli_stmt
instance)
- Query results (Again:
PDOStatement
instance or a mysqli_result
instance).
Trying to wrap all of this into a single class is a gross violation of the Single Responsibility Principle (SRP) - The S in S.O.L.I.D.. This principle states that a class can have only one reason to change. Which is a fancy way of saying that a Class can do only one job. Connecting to the DB, and representing the state of that connection is an important job. What results are obtained through that connection, and what happens to them afterwards is another job, hence it requires another class.
So what now?
To be honest, there's a lot of alternative routes you can explore. For now, though, I think the simplest one is to simply use data models, and a data provider, or mapper of sorts. Something like this might work quite well for you now:
class Dataprovider
{
/**
* @var PDO
*/
protected $db = null;
/**
* Pass a PDO connection to the constructor, this connection will be used to query the DB
*
* @param PDO $db
*/
public function __construct(PDO $db)
{
$this->db = $db;
}
/**
* Pass a model instance to this method, and a bind array to get the data
* if the bind is null, all records will be fetched
*
* @param Model $model
* @param array $bind = null
* @return array
*/
public function fetchModels(Model $model, array $bind = null)
{
$fields = $model->getFields();
$query = sprintf(
'SELECT %s FROM %s.%s',
$model->getFields(),
$model->getDbName(),
$model->getTable()
);
if ($bind !== null) {
$where = [];
foreach ($bind as $k => $v) {
//perhaps implement bind_param here, to specify types, for now KISS:
$where[] = sprintf('%s = :%1$s', $k);
}
$query .= ' WHERE '.implode(' AND ', $where);
}
$stmt = $this->db->prepare($query);
$stmt->execute($bind);
//get the model class, so we can create the instances using the results
$class = get_class($model);
$models = [];
while ($row = $stmt->fetch(PDO::FETCH_ASSOC)) {
$models = new $class($row);
}
return $models;
}
}
Of course, this'll require you to (re)write the models, too:
abstract class Model
{
protected $table = null;
protected $dbName = 'default';
protected $fields = [];
public function __construct(array $data = [])
{
foreach ($data as $key => $value) {
if (in_array($key, $this->fields)) {
$setter = 'set'.ucfirst($key);//use setters to validate data
$this->{$setter}($value);
}
}
}
public function getDbName()
{
return $this->dbName;
}
public function getTable()
{
return $this->table;
}
public function getFields($asArray = false)
{
return $asArray === true ? $this->fields : implode(', ', $this->fields);
}
}
//an example:
class User extends Model
{
const STATUS_ACTIVE = 1;
const STATUS_UNCONFIRMED = 0;
const STATUS_DELETED = -1;
protected $table = 'user';
protected $fields = [
'id',
'status',
'email',
];
private $id,
$status,
$email;
public function setStatus($status)
{
//validate: if ($status !== self::STATUS_ACTIVE && ...){}
$this->status = $status;
return $this;
}
public function getStatus()
{
return $this->status;
}
public function setId($id)
{
if (!is_numeric($id)) {
throw new InvalidArgumentException('A user id is numeric');
}
$this->id = (int) $id;
return $this;
}
public function getId()
{
return $this->id;
}
public function setEmail($email)
{
if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
throw new InvalidArgumentException('Invalid email!');
}
$this->email = $email;
return $this;
}
public function getEmail()
{
return $this->email;
}
}
With these classes, you could quite easily query the DB like so:
$activeUsers = $dataProvider->fetchModels(
new User(),
[':status' => User::STATUS_ACTIVE]
);
foreach ($activeUsers as $user) {
echo 'User email ' . $user->getEmail() . ' is active and, thanks to the setter, Valid', PHP_EOL;
}
Note that this code just an example. It's far from complete/finished. These classes were also written in one go, here, on this site, so I haven't actually bothered to test/lint-check any of the code. It's possible the syntax isn't quite right here and there
mysql_*
extension. It's no longer being maintained. Learn to use the replacement extension(s):PDO
andmysqli
, and learn to love prepared statements \$\endgroup\$