I ran the usernode module through the Coder module's style checker, and had a few other style, apidox & consistency fixes. Short overview:
- Have the apidox conform to the guidelines that were recently discussed on the mailing list
- Apply all suggestions from Coder
- A few spelling fixes
- Don't use by-reference parameters where the parameters aren't changed
- I couldn't do trailing space removal as usernode.module was already clean before :D
| Comment | File | Size | Author |
|---|---|---|---|
| usernode_style_fixes.patch | 14.9 KB | jpetso |
Comments
Comment #1
fagothanks.
> - Don't use by-reference parameters where the parameters aren't changed
I've done this to speed-up things. So that big objects (like nodes) aren't copied, if it's not needed. Is there some guideline to not do this, that I've missed?
Comment #2
jpetso commentedWell you're right, there's no specific guideline on this. So, let's have a pass-by-reference smackdown...
Pro:
- Performance with PHP 4 is indeed better, as objects are still copied there.
Contra:
- Readability is better when arguments are only passed by reference if they are actually modified. When doing it this way, you can recognize at a glance if the argument will be modified or not. (That's the main reason for me doing this, and also why I included it here as "style fix".)
- Performance with PHP 5 is at least as good without explicit passing-by-reference, as objects are always passed by reference there. I found some good coverage here (third post).
- Drupal core itself (which places much importance on optimizing performance) doesn't use pass-by-reference when the object isn't altered. Examples: hook_access, hook_link, node_show, or user_view.
Perhaps it makes sense to wait until PHP 4 isn't supported anymore. But then, everyone who needs peak performance will likely upgrade to PHP 5 in the first place. You decide.
Comment #3
fagoyour link to webmasterworld doesn't work, however afaik objects are passed by reference per default with php5. drupal core doesn't use it always, but sometimes e.g.: forum_form, taxonomy_node_validate, .. but I agree with your first point.
However, I don't want to introduce own guidelines for the nodeprofile modules - this would make accepting patches not so funny.. ;) for this we have the drupal guidelines. So I'm ok with your proposed handling of & as it increases readability - but I will not force possible contributers to follow it..
I've committed your patch to head and 5.x. thanks :)
Comment #4
(not verified) commented