|
|
| (76 intermediate revisions by 14 users not shown) |
| Line 1: |
Line 1: |
| == PHP5 constructor == | | <p class="note">Note: Please avoid using this talk page to propose changes to the coding style. Instead [https://tracker.moodle.org/secure/CreateIssueDetails!init.jspa?pid=10020&issuetype=1&components=12132&summary=coding%20style%20change file a coding style bug]. See the forum discussion [http://moodle.org/mod/forum/discuss.php?d=202406 Using Moodle: Proposing changes to moodle coding style] for more details.</p> |
| | |
| Should we enforce the PHP5 constructor __construct() instead of $classname() [[User:Nicolas Connault|Nicolas Connault]] 19:55, 19 May 2009 (UTC)
| |
| :+1. I think we agreed about that some time ago (when igniting some 2.0 developments) --[[User:Eloy Lafuente (stronk7)|Eloy Lafuente (stronk7)]] 00:03, 20 May 2009 (UTC)
| |
| | |
| == Inline comments ==
| |
| | |
| I have been using since ages ago "///" with outer alignment (I think that it was caused by some agreement long time ago, not my decision), for example:
| |
| | |
| <code php>
| |
| function check_moodle_environment(..., ..., ..., ...) {
| |
| | |
| $status = true;
| |
| | |
| /// This are cached per request
| |
| static $result = true;
| |
| static $env_results;
| |
| static $cache_exists = false;
| |
| | |
| /// if we have results cached, use them
| |
| if ($cache_exists) {
| |
| $environment_results = $env_results;
| |
| /// No cache exists, calculate everything
| |
| } else {
| |
| /// Get the more recent version before the requested
| |
| if (!$version = get_latest_version_available($version, $env_select)) {
| |
| $status = false;
| |
| }
| |
| ....
| |
| ....
| |
| </code>
| |
| Is that allowed, or only the "//" with inner alignment as commented at [[Coding style#Inline comments|inline comments]] --[[User:Eloy Lafuente (stronk7)|Eloy Lafuente (stronk7)]] 00:03, 20 May 2009 (UTC)
| |
| | |
| | |
| I did start doing all mine this way, but if you look at the code it seems you and I are the only ones. :P It seems sensible to make the standard fit what most people are used to (and how most of the code looks).
| |
| -- [[User:Martin Dougiamas|Martin Dougiamas]] 06:02, 9 June 2009 (UTC)
| |
| | |
| | |
| Eloy and Martin, this the only thing which consistently offends me about moodle coding style! Please go inline like all the cool kids ;-) --[[User:Dan Poltawski|Dan Poltawski]] 00:15, 12 June 2009 (UTC)
| |
| | |
| LOL. +1 for inline in even lines + out in odd one :-P
| |
| (seriously np with inline at all, it's only one habit easily modifiable) --[[User:Eloy Lafuente (stronk7)|Eloy Lafuente (stronk7)]] 01:03, 12 June 2009 (UTC)
| |
| | |
| ==Trailing spaces== | |
| | |
| "Lines should not contain trailing spaces. In order to facilitate this convention, most editors can be configured to strip trailing spaces, such as upon a save operation. However, if you are editing an existing Moodle file and are planning to submit your changes to CVS, please switch off that feature so that whitespace changes do not pollute CVS history (other developers will have trouble viewing what you've done if every other line has been edited for whitespace)."
| |
| Wouldn't the CVS history only get 'polluted' when someone fetched a file that DIDN'T follow this principle and returned it following the principle? Isn't that the price that has to be paid to clean things up? Otherwise this is pretty scary for someone who feels: "Great I can turn on my editor to autostrip those when saving. BUT I have to remember to turn that off if submitting it! (Or become a 'polluter'!)"
| |
| And what happens if I have already saved it locally while working on (and striped those trailing spaces) and later try to commit it. Maybe my ignorance about CVS is making me pose a "silly" question. [[User:Jeff Forssell|Jeff Forssell]] 07:17, 27 May 2009 (UTC)
| |
| | |
| : Jeff, the point with coding guidelines is that all new code must follow all the guidelines. With old bad code, mostly it is better to make the smallest change necessary when, for example, you fix a bug. That makes it clearest what the purpose of your change was. However, any line you do change while fixing the bug, you can then improve the coding style on that line.
| |
| | |
| : Really good editors actually have an option "Strip trailing whitespace when saving a file - but only from lines that I have edited", which is what you really want.
| |
| | |
| : The real way to avoid being a CVS polluter is to follow the best practice and always do a cvs diff and review all your changes (and if necessary amend them) before you do a CVS commit.--[[User:Tim Hunt|Tim Hunt]] 03:52, 1 June 2009 (UTC)
| |
| | |
| === Breaking up lines (e.g. huge arrays) ===
| |
| | |
| The coding style says to align the start of the following lines with the element in the first.. e.g.
| |
| | |
| <code php>
| |
| $myarray = array( 'fred' => 'blue',
| |
| 'tom' => 'green' );
| |
| </code>
| |
| | |
| I have to say I don't like this. While it looks very pretty it is a code maintenance nightmare. If you change (say) the array name you have to change every other line just to keep it looking nice. Surely if you '''must''' have it looking like this, do this...
| |
| | |
| <code php>
| |
| $myarray = array(
| |
| 'fred' => 'blue',
| |
| 'tom' => 'green' );
| |
| </code>
| |
| | |
| It's the same issue with long lists of assignments - making all the RHSs line up. Completely unnecessary and just asking for errors. Don't encourage people to change code that they don't have to. If they don't touch it then they can't break it. --[[User:Howard Miller|Howard Miller]] 13:52, 1 June 2009 (UTC)
| |
| | |
| :Agree 100% with Howard, I prefer the 2nd alternative for sure, if not always, at least in places where indentation is already considerable. BTW, same for harcoded SQLs and other strings. --[[User:Eloy Lafuente (stronk7)|Eloy Lafuente (stronk7)]] 13:58, 1 June 2009 (UTC)
| |
| | |
| : +1 from me too for a fixed indent for follow-on lines. Actually I have always used 8-spaces (double the normal indent) ever since I started programming in Java and read their docing guidelines. I think it is helpful to have a different level of indent for a follow-on line and a block.
| |
| | |
| : Sure, makes sense. I've fixed the docs. The only exception IMO would be wrapping function parameters, which just look too wierd to me if you wrap them the same as arrays. See [https://docs.moodle.org/en/Development:Coding_style#Wrapping_function_declarations the example]. -- [[User:Martin Dougiamas|Martin Dougiamas]] 06:08, 9 June 2009 (UTC)
| |
| | |
| : At the expense of some white space...
| |
| | |
| <code php>
| |
| public function graded_users_iterator(
| |
| $course,
| |
| $grade_items = null,
| |
| $groupid = 0,
| |
| $sortfield1 = 'lastname',
| |
| $sortorder1 = 'ASC',
| |
| $sortfield2 = 'firstname',
| |
| $sortorder2 = 'ASC') {
| |
| </code> | |
| | |
| : There is an argument that a function is badly thought out if it needs line after line of parameters. Won't go there though :-) --[[User:Howard Miller|Howard Miller]] 09:51, 23 June 2009 (UTC)
| |
| | |
| == Function and method declaration ==
| |
| | |
| [[Coding_style#Function_and_method_declaration]] says "Don't leave spaces between the function name and the opening parenthesis for the arguments." and yet the example below it completely disregards it. Please fix it. --[[User:Kumaraguru G|Kumaraguru G]] 21:32, 4 July 2009 (UTC)
| |
| : Hi, Kumaraguru. This is a wiki so you can just do it yourself ;-) --[[User:Frank Ralf|Frank Ralf]] 21:45, 4 July 2009 (UTC)
| |
| ::Hi Frank, the page is protected and I don't have edit access :) --[[User:Kumaraguru G|Kumaraguru G]] 00:30, 5 July 2009 (UTC)
| |
| :::I just fixed that, and a few other things I noticed.--[[User:Tim Hunt|Tim Hunt]] 03:13, 5 July 2009 (UTC)
| |
