[wp-hackers] We already know you are awesome, you don't have to prove it

[Jacob Santos]

Perhaps. I do agree that having someone else look over your code is a 
great way to find obvious mistakes.

There aren't too many places that I've seen where there is a "WTF?" 
moment. Digging through patches for me, is whether said patch would make 
my life as a plugin writer easier or harder. If it is the latter, then 
you'll know I'll be kicking and screaming about its inclusion.

I usually write overly complex code while I'm not tired and usually 
don't realize it until I go to fix a mistake. The problem with the 
referenced logic is that it is confusing based on the precedence. 
However, I first read the '=' as '==' as that is what I expect to see in 
an IF expression. Looking at it, I immediately questioned, "WTF? Does 
this check to see if the variable is an array? BY TYPECASTING!!!! 
Nooooo!" If you can write code like that at 2am, then I bow to you oh 
great one! Seriously, it is l33t code, but it is not needed because 
is_array() exists for the purpose of checking if something is an array 
and you won't confuse people who don't know about type casting.

Still working on the patch however. There are a lot of foreach arrays 
and not all of them need to be type casted to an array.

Peer reviews would be good for already existing code, well for any code. 
I do think that peer reviews are more for obvious bug issues and "WTF?" 
moments. For myself personally, I think the foreach issue should be 
fixed by first evaluating whether the variable is an array using 
is_array() before starting the array, but that is just me. I'm unsure if 
you type cast if the foreach will silently fail if it sees that the 
count is 0 as it would be if you type cast something that isn't an 
array, "What, there aren't any elements in this array. Failure!" I've 
never had a chance to test my theory however. I do assume that if you 
type cast something that it will at least run the loop once, but again 
I've never had the opportunity to test my theory. Sanity checks are 
always good however.

Oh yeah. The first reference was a for loop and not a foreach (had 
foreach on my mind). Did you know that in C#, you are not allowed to 
manipulate the array you are traversing? Wild. Did you know that in PHP 
you are technically not supposed to manipulate the array you are 
traversing in a foreach loop, but PHP will not crash and burn like C# 
will? Good stuff to keep in mind.



Robin Adrianse wrote:
> I'm pretty sure (well, I hope this is the case) that this was all part of
> the "2 am syndrome" -- looking at jumbled code so long it starts to make
> sense. It may be a good idea to enforce peer review for big patches, much
> like how the Webkit (and others) project does it, so the chance of these
> weird pieces of code can be caught and ironed out. I often can't find
> mistakes in my code, but it takes only a few seconds for others to find
> simple mistakes.
>
> On Jan 5, 2008 9:43 PM, Jacob Santos <wordpress at santosj.name> wrote:
>
>   
>> Can we keep overly complex code out of the source when it does not need
>> to be?
>>
>> I've noticed while loops that should be foreach loops (fixed). I've just
>> seen code that tests whether a variable is an array without using
>> is_array() (will be fixed in patch when finished).
>>
>> Yes, you have proven that you are awesome, but you don't need to prove
>> it. Now for the love of all that is holy please stop. Overly complex
>> code when it does not need to be does not help people decipher your code.
>>
>> Thank You.
>>
>> --
>>
>> Jacob Santos
>>
>> http://www.santosj.name - blog
>>
>> Also known as darkdragon and santosj on WP trac.
>>
>> _______________________________________________
>> wp-hackers mailing list
>> wp-hackers at lists.automattic.com
>> http://lists.automattic.com/mailman/listinfo/wp-hackers
>>
>>     
> _______________________________________________
> wp-hackers mailing list
> wp-hackers at lists.automattic.com
> http://lists.automattic.com/mailman/listinfo/wp-hackers
>   


-- 

Jacob Santos

http://www.santosj.name - blog

Also known as darkdragon and santosj on WP trac.