[RFC|Rejected] Do not use sizeof to check for empty arrays a

These RFCs were either rejected or have been replaced by an alternative proposal. They will not be included in phpBB.
Post Reply
User avatar
brunoais
Registered User
Posts: 964
Joined: Fri Dec 18, 2009 3:55 pm

[RFC|Rejected] Do not use sizeof to check for empty arrays a

Post by brunoais »

I'm requesting 2 changes in the coding guidelines.
The changes are simple. They are supposed to improve phpbb's performance without changing its functionality.
No1:
Instead of using:

Code: Select all

if (sizeof($some_array))
Use:

Code: Select all

if (empty($some_array))
The reason behind this is that sizeofis O(n) and requires more testing and working. empty() is optimized to do just the simple test of "is array? If it is, is it empty?"

Some small tests using 2 while loops (one for each one of this situations) and 5000 cycles shows that:
0.21661281585693
vs
0.055058002471924

And a test without while loop show (this is the most favorable result to sizeof):
9.9897384643555E-5
vs
2.8133392333984E-5 <- May I say... This number is very stable between tests.

No2.

Code: Select all

if (strpos($string, '?') === 0)
Where '?' is any string composed by 1 character.
Use:

Code: Select all

if ($string[0] === '?')
The reason behind this is that strpos is O(n) and is a function (2 penalties). Accessing the substring of 1 character in a string where we know where the caracter is, is faster if you use the string as an array instead of using the strpos().

Some small tests using 2 while loops (one for each one of this situations) and 5000 cycles shows that:
0.30220890045166
vs
0.12246012687683

And a test without while loop show (this is the most favorable result to strpos):
8.0108642578125E-5
vs
2.6941299438477E-5 <- May I say... This number is very stable between tests.


These two tests show that what I'm suggesting is a faster way of executing the code than the way it appears in many places in the current phpbb's source code.

The changes I'm requesting is based on these sentences:
- sizeof() is a function used to check the size of an array or a string, it's not supposed to be used to check if an array has elements. There are better ways of doing that at the moment.
E.g.
These are wrong:

Code: Select all

if (sizeof($array))
if (sizeof($array) > 0)
This is right:

Code: Select all

if (!empty($array))
This is also right:

Code: Select all

if (sizeof($array) > 1)
same as this:

Code: Select all

if (sizeof($array) > 30)
- strpos is a function to find where the start of a substring exists in a string. If the string you are comparing has, at least, the size of the position of the character where you want to test and you know that the string given is not supposed to be the empty string then use the equality of two strings. E.g
These are all wrong:
If you know the string has, at least, 1 character

Code: Select all

if (strpos($string, '!') === 0)
if (strpos($string, 'g') === 0)
If you know the string has, at least, 5 characters

Code: Select all

if (strpos($string, '6') === 5)
if (strpos($string, 'j') === 5)
These are right:

Code: Select all

if (strpos($string, 'abc') === 0)
if (strpos($string, 'atl') === 3)
These are the replacements for the wrong ones (in order):
If you know the string has, at least, 1 character

Code: Select all

if ($string[0] === '!')
if ($string[0] === 'g')
If you know the string has, at least, 5 characters

Code: Select all

if ($string[5] === '6')
if ($string[5] === 'j')

What do you say about this? Comments are welcome (well... this is an RFC, anyway)!
Edit: After some attention calls, I made some extra stuff.
Last edited by brunoais on Sun Feb 19, 2012 1:23 pm, edited 1 time in total.

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Do not use sizeof to check for empty arrays and...

Post by naderman »

This is a micro optimisation that is going to save so little time, it's not even worth the amount of time you spent writing this RFC, please focus on meaningful changes, that users will be able to benefit from.

User avatar
bantu
3.0 Release Manager
3.0 Release Manager
Posts: 557
Joined: Thu Sep 07, 2006 11:22 am
Location: Karlsruhe, Germany
Contact:

Re: [RFC] Do not use sizeof to check for empty arrays and...

Post by bantu »

I do think that !empty() should be used over sizeof() when checking for an empty array. However, I agree with naderman that this is not worth it for the existing codebase. Also, sizeof() on arrays is O(1), because arrays store their size.

igorw
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC] Do not use sizeof to check for empty arrays and...

Post by igorw »

What Nils and Andreas said.

User avatar
callumacrae
Former Team Member
Posts: 1046
Joined: Tue Apr 27, 2010 9:37 am
Location: England
Contact:

Re: [RFC] Do not use sizeof to check for empty arrays and...

Post by callumacrae »

naderman wrote:This is a micro optimisation that is going to save so little time, it's not even worth the amount of time you spent writing this RFC, please focus on meaningful changes, that users will be able to benefit from.
+1
Made by developers, for developers!
My blog

User avatar
MichaelC
Development Team
Development Team
Posts: 889
Joined: Thu Jan 28, 2010 6:29 pm

Re: [RFC] Do not use sizeof to check for empty arrays and...

Post by MichaelC »

naderman wrote:This is a micro optimisation that is going to save so little time, it's not even worth the amount of time you spent writing this RFC, please focus on meaningful changes, that users will be able to benefit from.
+1. Its like the difference between the speed of echo and print.
Formerly known as Unknown Bliss
psoTFX wrote: I went with Olympus because as I said to the teams ... "It's been one hell of a hill to climb"
No unsolicited PMs please except for quotes.

Post Reply