is_null is mentioned in various files and modules. !isset is well known to be faster than is_null and should be usedd with variables. This patch replaces all the occurrences of is_null in the base system. More patches would follow soon.

CommentFileSizeAuthor
#10 base-isset_is_faster_than_is_null-issue921606-10.patch9.93 KBmunzirtaha
#8 isset.patch9.42 KBbleen
#6 base-isset_is_faster_than_is_null-issue921606-6.patch9.42 KBmunzirtaha
base-isset_is_faster_than_is_null.patch2.59 KBmunzirtaha
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented
Issue tags: +Performance

Added Tags to my related patches.

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented
Assigned: Unassigned » munzirtaha
sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Closed (duplicate)

Duplicate of #921582: is_null should be replaced by !isset because it's faster

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented
Status: Closed (duplicate) » Needs review

@sun: please don't close it as a duplicate. Though the title is the same, the patch is different. It's patching different set of files. I split the patches so it won't be huge and obseleted quickly by other patches before being committed.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

No need to split into different patches. Please merge that other patch into this one. Thanks!

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented
FileSize
base-isset_is_faster_than_is_null-issue921606-6.patch9.42 KB

I merged the two patches somehow. Hope, it 's in good shape now.

Status: Needs review » Needs work

The last submitted patch, base-isset_is_faster_than_is_null-issue921606-6.patch, failed testing.

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Needs work » Needs review
FileSize
isset.patch9.42 KB

you had a !$isset stuck in there

this patch is a straight find/replace on the patch in #6

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented

Thanks bleen18, yes I made a typo ;)

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented
FileSize
base-isset_is_faster_than_is_null-issue921606-10.patch9.93 KB

Rerolling the patch after fixing another small typo.

ddorian’s picture

ddorian CreditAttribution: ddorian commented

this was commited yesterday http://drupal.org/node/921582#new
is this duplicate?

munzirtaha’s picture

munzirtaha CreditAttribution: munzirtaha commented

@ddorian: No, it was not a duplicate and it's not committed yet. That one was a patch to the database component only where this one would patch all the other remaining places.

bleen’s picture

bleen CreditAttribution: bleen commented
Title: is_null should be replaced by !isset because it's faster! » another issue about replacing is_null with !isset because it's faster!

maybe this will make things a bit clearer ;)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: another issue about replacing is_null with !isset because it's faster! » Various code uses is_null() instead of !isset() #2
Issue tags: +Novice

Better title. No time to review right now, but I think that everyone is able to review this patch.

bleen’s picture

bleen CreditAttribution: bleen commented
Status: Needs review » Reviewed & tested by the community

I've looked over this patch a couple times ... RTBC

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Performance, -Novice

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Performance, +Novice

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.