I've finally put together some commandments, er, Coding Guidelines that explain a few things and set out some rules that have been "invisible" until now.
It's still growing, but I'd appreciate any feedback on the clarity of what's in there as well as suggestions for additions before the translators start on it.
http://moodle.org/doc/?frame=coding.html
Its very useful.
The only slight point I think I would like to make is about variable names. I have spent years trying to get people to user longer variable names. Silly long names could be described as a good fault.
I personally have always subscribed to the Java style:
$thisIsAVeryLongVariable
but
$this_is_a_very_long_variable
is also good if harder to type; however
$thisisaverylongvariable
is no good, and I can see where you are coming from if you take the all lower case approach.
I strongly believe that good descriptive, readable variable names go a long way towards generating easy to understand code.
Don't hate me
Thanks, Howard. I'm in total agreement that names should be descriptive and readable. 
In practice Moodle code is mostly dealing with arrays of objects, and the context is obvious, so one or two words has almost always been enough.
For example, if dealing with assignment submissions, there is no need to name the array $assignmentsubmissions, since it's ALL assignment stuff .. so just $submissions works fine, and "reads as English" very well, eg:
foreach ($submissions as $submission) {
$submission->marker = $user->id;
}
That said, if you really NEED to make a longer name by all means do so, as long as it reads well.
In practice Moodle code is mostly dealing with arrays of objects, and the context is obvious, so one or two words has almost always been enough.
For example, if dealing with assignment submissions, there is no need to name the array $assignmentsubmissions, since it's ALL assignment stuff .. so just $submissions works fine, and "reads as English" very well, eg:
foreach ($submissions as $submission) {
$submission->marker = $user->id;
}
That said, if you really NEED to make a longer name by all means do so, as long as it reads well.
Which brings up another point... do you want to add a comment about use of plurals? Something along the lines of don't, unless its an array of some sort, giving you the option of using the singular as an array location.
Or something...
Or something...
A very useful Guide. A few points:
I may be wrong but one important rule/style I keep to is the use of objects in function arguments. I'm working to the unspoken rule/style "If there's a choice between sending a function a dirt great object instead of a nice neat integer send in the object!" Now that's rather an extreme way of expressing it
The positive side is much much neater code, room for expansion in the function as it's got all the gubbings, and less hits on the database.
My impression is that in the Moodle development things started by passing integers, for example isstudent($course->id... and evolved to passing objects, for example print_table($table... But then I may be reading too much into it.
However, if it IS a rule/style it's a jolly good one, worth documenting and worth following. If it's NOT a rule/style I've got several thousands of lines of code to rehash
Oh I'm not sure whether its a rule or a style
- In rule 12, let's hear it for one of my favourite functions - print_heading_with_help
In the example of Style 4, doesn't the statement
} else if (empty(...
break style rule 5? and should be
} elseif (empty(...
Nobody can say we don't read the Documentation
1. Yes, there is some argumentative tension there. I usually pass the whole object, unless the function is trivial (like isstudent). Also, sometimes you don't HAVE the whole object and it would be time consuming to fetch it just to pass it and not use it. So use your judgement. I believe in PHP5 everything will be passed by reference all the time so the efficiency question will be moot.
2. Next will be a complete API reference using phpdoc (Russell has already done some work for datalib.php).
3. Wow, that's a sharp eye but "elseif" and "else if" are synonyms aren't they? I always use "else if" anywhow.
2. Next will be a complete API reference using phpdoc (Russell has already done some work for datalib.php).
3. Wow, that's a sharp eye
API -documentation can be made quite fast and easy with PHPEdit (http://www.phpedit.com/).
This fine editor has a tool that I fell in love right a way, The Help Generator, which makes quite fine documentation.
I just made some example docs in http://edu.jaiko.fi/~janne/moodledocs
Username: moodle
Password: moodledev
Check it out
This fine editor has a tool that I fell in love right a way, The Help Generator, which makes quite fine documentation.
I just made some example docs in http://edu.jaiko.fi/~janne/moodledocs
Username: moodle
Password: moodledev
Check it out
Janne,
That is nice...thanks.
I wonder if there's any way to get it locally or put it up on a server, maybe sourceforge.net?, where it could be accessed as needed?
I'd use it here locally, but I'd hate to use up your server resources with miscellaneous hits unnecessarily.
Thanks, Janne.
Les.
Very pretty - and even better it seems to be using the phpdoc info! Thanks for the tip!
(Phpdoc comments were only done for datalib.php so far because I suggested to Russell there might be many cleanups in the other libraries for Moodle 2 and so not to bother yet ... comments in phpdoc form are still necessary to make these phpedit docs useful)
Still, even the function name and arguments is useful - I'll put these up somewhere in moodle.org.
(Phpdoc comments were only done for datalib.php so far because I suggested to Russell there might be many cleanups in the other libraries for Moodle 2 and so not to bother yet ... comments in phpdoc form are still necessary to make these phpedit docs useful)
Still, even the function name and arguments is useful - I'll put these up somewhere in moodle.org.
Martin, I'd love to see a new section in the coding guidelines on phpdoc documentation. Developers with cvs access could add little bits of phpdoc info as they edit files. Developers should be encouraged to add properly formatted, descriptive, phpdoc comments as they add new code/files.
I'd love to see something like section 6 of OpenClinic's guidelines:
http://openclinic.sourceforge.net/openclinic/coding_guidelines.html
For some info on what options can be used in phpdoc comments see:
http://www.phpdoc.org/docs/HTMLSmartyConverter/default/phpDocumentor/tutorial_phpDocumentor.howto.pkg.html#documenting.elements
I'd love to see something like section 6 of OpenClinic's guidelines:
http://openclinic.sourceforge.net/openclinic/coding_guidelines.html
For some info on what options can be used in phpdoc comments see:
http://www.phpdoc.org/docs/HTMLSmartyConverter/default/phpDocumentor/tutorial_phpDocumentor.howto.pkg.html#documenting.elements
While on this topic I have another suggestion - I noticed this in calendar code and have made a note to try emulating:
if reading int value arguments ensure that the data is valid, especially before inserting into the database
$day = intval($_GET['cal_d']);
vs.
$day = $_GET['cal_d'];
Pretty basic things like this and perhaps the proper times to use p() and s() from weblib would be good guidelines in my opinion.
if reading int value arguments ensure that the data is valid, especially before inserting into the database
$day = intval($_GET['cal_d']);
vs.
$day = $_GET['cal_d'];
Pretty basic things like this and perhaps the proper times to use p() and s() from weblib would be good guidelines in my opinion.
These guidelines are very helpful. I would like some similar guidelines regarding the database. I think I have gathered the following by looking at current practice:
- Every table shall have an id (INT(10)) field as primary index and have auto_increment turned on.
- The table containing instances of a module shall have the same name as the module, e.g., newmodule.
- Other tables associated with a module and holding information about things shall be called newmodule_things (notice the plural in things).
- Table fields that hold the id number of entries in another table, let us call it object shall either be called object or objectid (Martin, do you have a preference here? Both seem to be used equally at the moment.)
- Boolean table fields shall be implemented as TINYINT(2) rather than as BOOLEAN, with 0 corresponding to false and 1 to true.
- name fields shall be of type VARCHAR(255).
- Tables shall have a timemodified (INT(10)) field and code updating the database is required to set this with a PHP timestamp obtained with time(). (Martin: which tables arer required to have a timemodified field and which ones don't?)
- All table fields shall have attribute NOT NULL. Zero or the empty string are to be used instead of NULL (Martin: is this really a rule and if so, why?)
Thanks, Gustav - good idea!
I've added database guidelines to the coding guidelines page.
The difference between object and objectid is that the latter is a newer standard to help self-documentation in the code. It also avoids some problems with reserved words (eg when PostgreSQL support was added I had to go through all the code and change "user" fields to "userid" just so it could work). All new tables should refer to id fields in other tables using objectid and eventually I suppose we might fix the old tables someday.
timemodified should go in almost all new tables, unless it really is never going to be useful there.
The NOT NULL thing is not a standard .. it's just what phpMYAdmin adds by default and it stuck. It shouldn't matter either way to PHP/Moodle, which treats all empty things as equal.
I've added database guidelines to the coding guidelines page.
The difference between object and objectid is that the latter is a newer standard to help self-documentation in the code. It also avoids some problems with reserved words (eg when PostgreSQL support was added I had to go through all the code and change "user" fields to "userid" just so it could work). All new tables should refer to id fields in other tables using objectid and eventually I suppose we might fix the old tables someday.
timemodified should go in almost all new tables, unless it really is never going to be useful there.
The NOT NULL thing is not a standard .. it's just what phpMYAdmin adds by default and it stuck. It shouldn't matter either way to PHP/Moodle, which treats all empty things as equal.
And if you create table names in your prototype, do not forget to use the prefix:
$CFG->prefix = 'mdl'; //user chooses his local prefix (in config.php)
I'm using your guidelines while trying to modify a Wiki application to fit into Moodle as a module.
One thing I've come against; I'm guessing that some of the Moodle code requires a field called 'name' as well? If this isn't true, how can I redefine my module to use a field called something other than 'name' as the display name?
mike
It also looks like you need to have a field called 'course', which contains the 'id' for the course that the module belongs to.
mike
That is a good point. The way the NEWMODULE template is at the moment it indeed assumes in mod.html that the field that contains the course id is called 'course'. This conflicts with the coding guidelines.
cvs:/moodle/mod/newmodule_template.zip deserves an overhaul anyway. For example it should contain a config.html.
Thanks, Mike - yes "course" and "name" are required for the main module tables. I've updated the guidelines to reflect this.
As Gustav said the use of "course" should really be "courseid" but since this convention came later the older core tables don't always use it (and fixing them now would be a HUGE effort). I've updated the docs to make this clearer too.
As Gustav said the use of "course" should really be "courseid" but since this convention came later the older core tables don't always use it (and fixing them now would be a HUGE effort). I've updated the docs to make this clearer too.
Mike, I don't think there is a requirement to have a 'name' field. You can store the display names of your module instances in any way you like because it is only the files from your module (like view.php, index.php, mod.html) that refer to it.
If an element shows up in the Main Menu, it uses the name field to display it. I'm not sure what part of Moodle displays the Main Menu. There may be other occurrences as well.
mike
Thanks for pointing that out. You are entirely right. There has to be a name field. One other place that uses the name field is the gradebook which I should have know since I just worked on that code 
.
.