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

CommentFileSizeAuthor
usernode_style_fixes.patch14.9 KBjpetso

Comments

fago’s picture

thanks.
> - 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?

jpetso’s picture

Well 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.

fago’s picture

Status: Reviewed & tested by the community » Fixed

your 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 :)

Anonymous’s picture

Status: Fixed » Closed (fixed)