database class review

[ngreenwood6]

I have spent a little time lately developing a database class (not finished yet) that automatically will escape the data that is sent to it if used properly. I would like to get some input on it and see what some of you guys think of it. Mainly I would like to know if it is easy to use, if there is any potential issues so far, and if there are any suggestions on better ways to do things in the class. I have attached a copy of the class to this post. Here is an example of how to use it:

 

//create object
$db = new db();

//perform query
$users = $db->table('users')->select('*')->where("name='%s'")->vars("somename")->getResults();

//another way to perform the query above
$db->table('users')->select('*')->where("name='%s'")->vars(array("somename"));
$users = $db->getResults();

 

You will have to change the db connection properties at the top of the db.class.php file for your own connections. Please let me know of any suggestions, questions or issues that you have.

 

[attachment deleted by admin]

[ignace]

Completely unnecessary:

 

define('OBJECT','mysql_fetch_object');
define('ASSOC','mysql_fetch_assoc');

 

And can be re-written to:

 

class Database {
    const FETCH_OBJECT = 'object';
    const FETCH_ASSOC = 'assoc';
    const FETCH_BOTH = 'both';
}

 

Also completely useless is:

 

define('DB_HOST','localhost');
define('DB_DB','mysite');
define('DB_USER','root');
define('DB_PASS','');

 

Not everyone uses constants, some like to use configuration files, environment variables, .. Your class although named db is mysql-specific, if I would want to shift from mysql to sqlite for example I would have to re-write most (if not all) of your methods. Plus in some cases (like my last project) I had to use mysql as my main database which was kept in sync with a program that ran the back-office and SQLite as a buffer to hold the most frequent executed queries to lower the load on the main mysql database.

[ngreenwood6]

Thanks for the reply. The constants that were declared were more or less just there so that I could just have it working for now to test the methods and such and will be removed. Like I said its not done yet and still has quite a bit of work to be put in. The main focus of this db class is mysql because it is the only database that I really ever work with. You never gave me your opinion on how easy the class was to use or what you thought of how it worked. I dont know if you noticed or not but you can keep building the query, so you can call a function more than once and it just builds onto it (the only one that doesnt is table because its not necessary). Here is an example:

 

$db->table('users')->select('*')->where('banned=0');
$db->where('AND enabled=1');
$db->getResults();

[phpfreak]

Just out of curiosity why not use a pre-existing database abstraction layer?  There's tons of them out there already that do the same things you're wanting to do here, but already written and optimized.

[ngreenwood6]

Haven't ever really found one. Can you give me some links to review?

[phpfreak]

Zend has some to use, also adodb use to be a pretty good one, not sure how well it's maintained these days.

[ngreenwood6]

I prefer to write my own stuff because I really dont like using 3rd party things. Plus I feel like when you create something yourself you have a better understanding of how it works and how to use it. Also this could be implemented on any site instead of having to use the zend framework.

[eran]

Writing your own is well and good and does has some learning value, but at least have a look at mature packages that have been developed and tested by many seasoned developers. You can learn a lot from reading the Zend_Db classes as phpfreak suggested

[ignace]

I prefer to write my own stuff because I really dont like using 3rd party things.

 

Remember that the software you write will not always be maintained by you. So whenever a new developer joins your team or someone takes over you maintenance work they won't have any documentation to refer to.

[Bodhi Gump]

Hi Greenwood,

 

My first impression? A lot of public methods, and very few private methods. It seems like you're not sure yet to what purpose you built this class and made it so that it can theoretically handle anything. This makes it very flexible, but you have to ask yourself why you would want to use this class over the standard php methods. Relying too much on public methods means that the object has to rely on other objects, which means that when you have to debug a problem related to this object, you have to consider more interactions between objects than is necessary for the task that needs to be done.

 

A general rule of thumb with object oriented programming, is that you want to make the interactions BETWEEN classes as simple as possible, making sure that other classes don't need to know in detail how the classes they interact with achieve their results.

 

What is 'as simple as possible'? Well, you may anticipate that you have to get records from the same joined tables over and over again in different parts of your application. Then it would make your life easier if all that is needed is a call such as get_user_logins($user_id) that either returns an array of records of user login information, or false on error. You don't want to worry about the connection with the database or the table names if at that part of your code, all you need is the login information of some user. If you then get an array of records belonging to the user you specified, splendid! Then you don't have to worry about the internals of your querying mechanics. If you get 'false', then you DO have to worry about that.

 

The bottom line is, that you have to keep in mind how you want to interact with your class (and how NOT to), so that your debugging time decreases because you know which class is supposed to do what, and your method calls are simple and direct.

 

Hope this helps.

[ngreenwood6]

@bodhi I dont think that you are understanding the point of this class. The point of this class is to create a database class that will automatically handle all of the database transactions that a user can give to it and return a dataset in the format that the user requires while sanitizing all of the data so the user does not have to worry about it. There are alot of public methods because they are required in order for the queries to be built. Essentially this class can be used as a standalone and will provide a good, easy database class to work with, or you could extend from this class and provide methods to get the data that you need in a simplified manner. so you could do:

 

class articles extends db(){
public function getArticles(){
     return $this->table('articles')->select('*')->where('id=1');
}
}

 

public methods are not a bad thing as long as you arent allowing the user to change anything that you wouldnt expect or want them to change.

[ignace]

and very few private methods.

 

Are you suggesting, he needs more private methods? Ever heard of a thing called Unit-Testing and the problem with private methods?

 

The point of this class is to create a database class that will automatically handle all of the database transactions that a user can give to it and return a dataset in the format that the user requires while sanitizing all of the data so the user does not have to worry about it.

 

Now you may think that you need MySQL for everything but if whatever you are building becomes a success you'll find your design is too inflexible. Let the DB handle DB specific stuff (connect(), disconnect(), prepare(), execute(), ..) and create Gateway classes that will hold an instance of your DB and let your application communicate with the Gateway this way you decouple your application from your DB, as you can modify the Gateway classes without having to touch your application code.

 

class UserGateway {
    private $db = null;
    
    function getAllUsers() {
        return new UserList($this->db->fetchAll('SELECT * FROM users'));
    }
    
    function findUserById($id) {
        return new User($this->db->fetchRow('SELECT * FROM users WHERE id = ..'));
    }
    
    function findUsersByLastName($lastName) {
        return new UserList(..);
    }
}

 

Also a good idea is to not return array() or int or stuff like that but instead return domain-specific classes the reason being that you can't modify the internal structure of array() or int but you can with domain-related classes.

[Bodhi Gump]

@Greenwood: Well if you put it like that, I suppose your class has the advantage of getting the data you want in a format that you need and can work with. Didn't mean to diss you, Greenwood. I respect the way you managed to recreate the mysql queries by making easy-to-read method call chains.

 

Are you suggesting, he needs more private methods? Ever heard of a thing called Unit-Testing and the problem with private methods?

Well, I do know that it takes a lot of time to get how a class works if that class is too 'intelligent'. Does the difficulty of Unit-Testing increase more with the number of private classes, with the size of the methods, or generally with the degree of autonomy of a class?

 

Now you may think that you need MySQL for everything but if whatever you are building becomes a success you'll find your design is too inflexible. Let the DB handle DB specific stuff (connect(), disconnect(), prepare(), execute(), ..) and create Gateway classes that will hold an instance of your DB and let your application communicate with the Gateway this way you decouple your application from your DB, as you can modify the Gateway classes without having to touch your application code.

Yes, I believe that's the way to go. Actually, shouldn't Greenwood's class  be regarded as an abstract class?

 

Also a good idea is to not return array() or int or stuff like that but instead return domain-specific classes the reason being that you can't modify the internal structure of array() or int but you can with domain-related classes.

Just to satisfy my curiosity: what kind of applications require domain-related classes to format the query results? Could you give me some hyperlinks?

[ignace]

Just to satisfy my curiosity: what kind of applications require domain-related classes to format the query results? Could you give me some hyperlinks?

 

Not format the query result (although possible) merely:

 

1) maintaining a healthy client contract (and not violating the encapsulation part of OO, ie the client doesn't need to know you work with array's),

2) make your design understandable (UserList says more then array),

3) make your design easier to change (you can't add a method to array).

[ignace]

Well, I do know that it takes a lot of time to get how a class works if that class is too 'intelligent'.

 

I am all for intelligent classes (instead of just dumb objects littered with get and set) as these are easier to maintain and keep your design simple/focused.

 

Does the difficulty of Unit-Testing increase more with the number of private classes, with the size of the methods, or generally with the degree of autonomy of a class?

 

You can't unit-test private methods. Your unit-test is also completely obsolete if the meat of your class is within private methods. You find the same problem with IF-structures these - although basic structures - make for multiple execution paths and thus a lower code coverage, the lower your coverage the more useless is your unit-test.

 

More on this in "What We Don't Need in OOP" by Giorgio Sironi (an extreme example with lots of answers) -- I also recommend you read his other articles.

[phpfreak]

The question to me is... Do you want to reinvent the wheel or try something tried and true?  If this class was for a unique purpose then it makes sense to invest time into it, but always remember that we are only offering advice on how to paint your canvas.. It's your artwork not ours... Paint it how you see fit!

[Bodhi Gump]

More on this in "What We Don't Need in OOP" by Giorgio Sironi (an extreme example with lots of answers) -- I also recommend you read his other articles.

Thanks!