Talk:DB layer 2.0 migration docs
- The developer reading this article MUST read XMLDB Documentation (at least Introduction + three first Developing section pages) otherwise he's going to ask plenty of questions :) It could be very good that the first line of this article be: "In order to understand this article you need to know how works the database abstraction layer in Moodle (previous to 2.0)".
- API link is broken (=> http://php.moodle.org/ "Server not found")
- We know that some changes need to be done on 2.0. But on what?
All these examples concerns the glossary module. It would be good to indicate it first. When I read it first, I didn't really know what was going to be explained in XMLDB changes. In fact all this section is about "Updating your upgrade.php file for Moodle 2.0". Conclusion: we really need a clear first sentence for the all "XMLDB changes" section. So we know what we are going to read. Once I knew what is about, all was very clear.
- If we don't make any change on DDL code we shouldn't make it as a section/block. The DDL section should be a note. If you remove this section don't forget to update: "Changes below are grouped into 3 main blocks" => "Changes below are grouped into 2 main blocks"
- DML section:
this is about to change all DML code, in all Moodle files? I think it miss an introduction line as well.
- Maybe you should use more the word: YOU should do that, YOU do this, YOU ... The developer will feel more guided.
Note: I wrote this comment on first read.
Note 2: I'm new as dev in Moodle, and I've just discover all the power of Moodle XMLDB reading. All the current XMLDB looks awesome, I don't even talk about the coming 2.0 :)
jerome mouneyrac 22:22, 22 May 2008 (CDT)
1 I added a marker re add_index or somesuch, is it really renamed add_key or is that just a copy/paste error? If it is correct, needs changing to emphasise that the function has really changed.
2 The section with nothing in it (DDL) should probably be removed :)
3 'Golden' and 'iron' makes no sense at all to me, nor did the explanation. Could we rename these in some way?
4 The example using named params should probably be changed to use better names for the params eg fn, ln - if you're using 'param1' and 'param2' you should use use the ordered one.
5 Some of the examples are unnecessarily wordy - for example, why put the array into a variable then call the function? It only has two elements, you can do it in the same line no problem.
- Wordiness in code samples is perfectly OK, Sam, but you're free to be more terse in your implementations ;) Nicolas Connault 06:59, 3 June 2008 (CDT)
- Yes, but my point is that it makes it look bad for people who are having to change. It's like, OMG! this was one line before and now it's four! wtf I have to define a separate array every time I scratch my backside! why are you making my life a misery, you unfeeling b****rds!!! That kind of thing. :) I don't think it makes it easier to understand to show use of a mostly-unnecessary separate variable. sam marshall 06:24, 6 June 2008 (CDT)
6 Wow this new system is a fantastic improvement (except for the new IN function which is kind of nasty)
sam marshall 04:32, 3 June 2008 (CDT)
Why / when do we have to use the params array in the "_sql" functions?
- I'm not sure I'm clear on what the advantages are in using the "params" array for the "*_sql" functions (e.g. get_records_sql). Also, the text says we must do it, but I see lots of examples in the already converted code where this hasn't been done. Is it really a must do or is it more of a can do?
- You must always use the params array WHEN you have params to pass ;) If the SQL doesn't contain dynamic parameters (subject to SQL injection), we don't need the params array. Nicolas Connault 13:57, 21 July 2008 (CDT)
- Aha! So the purpose is to help make sure that dynamic data used in SQL searches is cleansed? Is that correct? Mike Churchward 21:31, 21 July 2008 (CDT)
- Yes, and to get rid of the troublesome stripslashes().
- The new recordset handling seems odd. We used to use the "rs_fetch_next_record($rs)" function, which as I understood it was optimized so that the entire db records contents weren't stored in the PHP array variable. Now we are replacing it with a "foreach ($rs as $record)". Doesn't that mean that the entire data query results are stored in a PHP variable again?
- The new recordset uses a new feature of PHP5, the Iterator interface. Foreach can then be used on the recordset. (See the bottom of the adodb doc for more info). Nicolas Connault 13:57, 21 July 2008 (CDT)
- Hmmm... Looks like I need to spend some time learning PHP5's features. So, in PHP5, the "foreach" construct unlocks a classes' interator functions if they exist? Mike Churchward 21:31, 21 July 2008 (CDT)
- Correct :) Nicolas Connault 22:24, 21 July 2008 (CDT)