Re: Re: DB_Table: Summary and Pre-Call
| From: | Alan Knowles | Date: | Wed, 07 Apr 2004 02:38:10 +0000 |
| Subject: | Re: Re: DB_Table: Summary and Pre-Call | ||
| References: | 1 2 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-27107@lists.php.net to get a copy of this message | ||
I'll give you a +1 = as long as you sort out a few bugs (which came up when I tried implementing alot of these features in dataobjects :)
- dates - you cant use strtotime - it is borked beyhond belief :)
try storing my birthday in 1969 in there ....
* Date is a good idea here - have a look DB_DataObject::***Value()
- It took me a long time to realize that a NON-NUL Date column being sent '' should use null - i spent ages getting 0000-00-00 or 1999-12-31 crap from it..
- defines - have a read of georges's performance talk. - these are evil :) defining define('DB_TABLE_COL_STRING', 'string'); etc. is very redundant.. - and very expensive.
(ideas - not required though...)
** it would be nice if you could have used the same bitwise integers for the types, as dataobjects - that way the suggestion below would mean that we could share the database creation code....
- i'd be tempted to move your create routine (and the big $GLOBALS['DB_TABLE']['type'] setting stuff into a DB_Table_Create class)
- The addFromElements/getForm .. etc. seem very redundant.. -
why not pass make DB_Table_QuickForm handle it all independant ...
$a = DB_Table_QuickForm::construct($mytable);
Rwe==
Hans Lellelid wrote:
Hi Paul, I'm not going to be able to vote on your proposal, as I don't have a PEPr account, but I wanted to comment on a few aspects of it. I should start by saying that I like it. It does have a fair bit of overlap w/ my Propel project, but I'm all about choice & I think it provides a light-weight alternative in many respects (plus Propel is itself based on another package - Torque - so it's not like overlap is any sort of objection).-- Can you help out? Need Consulting Services or Know of a Job? http://www.akbkhome.comThis is one of the primary conceits of DB_Table: that you can forcethe database to store things the way you want them to be stored, thus avoiding the need to convert back-and-forth between native databasetypes when moving from one RDBMS to another. (C.f. the comments from the SQLite guys on their implementation as well.)I'm not sure I agree with (or perhaps completely understand) the reasoning behind this. Of course I think that the calling code shoudln't need to concern itself with how dates are stored in the DB -- e.g. 01/32/2002 14:55:33 in SQL Server or 20020132145533 in MySQL (yuk!) -- but actualy storing dates as text (and in general using a text type for storing non-text data) seems to defeat the pupose of using a database. (Or, to address your C.f., of using a non-SQLite database.) I believe that the native DB types should be used so that you can take advantage of (e.g.) date-time function in the database. Of course using such functions would probably tie your class to a particular RDBMS, but I think it should be possible if you really want to do this. More generally, I think that doing this makes databases designed for use with DB_Table to be non-standard or even idiosyncratic --- and this seems to be the opposite of what you want. Now .. I'm unsure whether it's DB_Table's job to deal w/ DB native types or whether it's DB's (or MDB's) job. I tend to think it's the latter, actually, and in my implementation I moved all of that logic down into the Creole level. I'm sure a good argument could be made for why the date parsing/formatting should be done in the upper levels, but I tend to think that the lowest level is the one that should have most intimate knowledge about quirks in the db.In addition, because the DB_Table instance has a defined column map,it knows what to expect from every field. Thus, it can pre-validate all INSERT and UPDATE values to make sure they match the column requirements (data type, size, decimal places, not-null, and so on) before attempting to connect to the database. This also allowsdevelopers to add customized validations for insert and update calls.This is nice. I actually like the option of doing the table definitions at runtime rather than a build phase (which is how Propel works); it opens up possibilities in the area of dynamic database structures that are interesting. I wonder if there's any reason, though, why DB_Table doesn't support primary key information? I'd think it would be fairly simple to have a save() method rather than an update() and insert() method. I assume you could use the native DB/MDB sequence emulation to generate the needed IDs, etc. (I personally don't like how these packages force you to emulate sequences instead of using the RDBMS native id generation, but since you are using DB/MDB you might as well use it, eh?) Those were my two main comments in looking over the package and briefly at the source. I apologize if I missed something that I should have seen. Obviously I don't think that you should change your code based on my comments, but I wanted to at least raise them for the sake of discussion. Nice work, Paul. Cheers, Hans