Can't I name it class.DBmysql.php ? Since the class itself isn't named mysql, it sounds a little confusing to use different names on the file and the class. I can understand if you want to use a prefix of class. infront of the classname, so everyone can see it's a class, but to rename it?
Ahwell, tell me what to do. Rename to class.mysql.php or class.DBmysql.php
I'd like to make continual little comments on the submitted code. Where do you guys think this would be best placed? Code Review is enabled for the project at Google so people can review lines (or files, blocks of code, etc.) even on a per-revision basis which would make things more tied together. Or, would those coding prefer comments to be made in threads like this one?
Already suggested it myself, but VI didn't want it for now.
Originally Posted by Tanax
Okey, after reading about their functions, I see why you thought it would sound silly or impractical. Indeed, it would be very impractical to have mysqli option in the mysql DB object. However, I've done a DB factory class, which would let you create a db object based off of what db you want, like:
$db = DB::getDatabase('mysql');
and it would then create a new DBmysql and return the instance.
It would then be easy to create other objects, such as DBmysqli, DBmysql_PDO, DBsqlite_PDO, etc.
But that's only if you guys want it. However, I think Orc will have to change the __construct in his class since it's checking if the class is an instance of DBmysql, and I don't know if someone else has done something using this class that would be affected by this change.
I would however be happy to change it, if you want.
And by "change it", I mean create the factory class.
Could I suggest using a DAL for the database? E.g. an abstract database class, that is extended by the MySQL class.
While I do not think that it is a bad idea, I don't want to get that complex with the SQL class right now. Reason being that all development is waiting for it, to add something else to it would delay development even more than it already has been,
Ah, it was a little more to it than that. I had to create a repository first, then download all the files, then modify the file, and THEN commit. But I've updated it now so it works with the getQueryResult(2, 'field')
Ah, it was a little more too it than that. I had to create a repository first, then download all the files, then modify the file, and THEN commit. But I've updated it now so it works with the getQueryResult(2, 'field')
Remember to do an SVN Update before you Commit to check that noone else has edited the file since you started :)
Tanax: I don't know if you noticed but I made a whole bunch of comments on your previous version, revision 55.
It's ok there's nothing too huge to worry about, I guess the main is that magic quotes line in the secure method. Also, there's the overall comment (which I'll quote to save you clicking) which could do with some attention.
Why are there so many places where one can provide a SQL query as an argument? I count 5.
There also appears to be a confusing selection of ways to achieve a usually simple task: grab an assoc. array of results. More clear documentation of the use of this class is requested (perhaps on the forums).