Code cleanup, removed a bunch of parser warnings and some actual undefined variables. Even though some might find this supurflous, I think it's good to keep the core code clean so we can spot those variables that are really undefined in the future.
These 4 are still questionable. I need some input on these:
The local variable $edit may not have been initialized user.module Drupal HEAD (Core)/modules/user line 1161
The local variable $form may not have been initialized search.module Drupal HEAD (Core)/modules/search line 245
The 2 below are still remaining, but I can see they should get set in the while. But the first time it runs through, they are undefined since $tag is FALSE.
The local variable $linknid may not have been initialized search.module Drupal HEAD (Core)/modules/search line 574
The local variable $linktitle may not have been initialized search.module Drupal HEAD (Core)/modules/search line 558
It's not a bad idea to flush these out, and real easy with PHPEclipse. I've caught some worthy mini-bugs just by loading up the project in Eclipse, might be a better solution than vi. :P
Comment | File | Size | Author |
---|---|---|---|
#4 | cleanup2.patch | 9.22 KB | RobRoy |
cleanup.patch | 8.1 KB | RobRoy | |
Comments
Comment #1
drummMost of these look okay. Except the
$null = NULL
. Just useNULL
.I'd like to see these broken up into separate patches and separate issues by file for easier reviewing.
Comment #2
RobRoy CreditAttribution: RobRoy commentedI thought the $null = NULL; was needed because some of those params are passed by reference.
Comment #3
drummMake a note of that in a comment and use NULL where arguments are not pass by reference.
Comment #4
RobRoy CreditAttribution: RobRoy commentedRevised patch for HEAD with changes suggested by drumm.
Comment #5
RobRoy CreditAttribution: RobRoy commentedComment #6
Jax CreditAttribution: Jax commentedTRUE, true, FALSE, false, NULL and null is used inconsistently throughout the code. Maybe there should be coding conventions for reserved keywords as well?
Comment #7
Jax CreditAttribution: Jax commented+1 for false, true, null for php (and NULL for SQL but that is mentioned).
Comment #8
drummNULL, FALSE, and TRUE are preferred.
Comment #9
RobRoy CreditAttribution: RobRoy commentedYeah, I've always used TRUE/FALSE as I prefer it and /thought/ that was the standard. That is why my additions are as such.
What is the Drupal coding standard for this? I see the PEAR page at http://pear.php.net/manual/en/standards.funcdef.php has a "true" so you may be right.
If one of the devs lets me know, I'll change the patch to reflect this.
Comment #10
RobRoy CreditAttribution: RobRoy commented@drumm: Okay cool.
Anything else needed to get this patch committed then?
Comment #11
RobRoy CreditAttribution: RobRoy commentedI suppose we should add some info on TRUE/FALSE/NULL to http://drupal.org/node/318 then.
Comment #12
Jax CreditAttribution: Jax commentedVerified the code and applied the patch, works.
Comment #13
drummCommitted to HEAD.
Please break your patches up into smaller chunks next time.
Comment #14
(not verified) CreditAttribution: commented