Talk:DB layer 2.0 migration docs

From MoodleDocs
  • 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)".
  • 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)

random things

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?

  1. 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().
  1. 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)