Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:24346
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
To: "'Matt W'" <php_lists@realplain.com>,
	<internals@lists.php.net>
Date: Tue, 11 Jul 2006 14:42:27 -0700
Message-ID: <002601c6a532$e8c91d40$a610a8c0@zend.2k>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 7bit
Thread-Index: AcaYSALg6zPjcpM6QbqbWIHq3MGhoQM6sCVg
In-Reply-To: <009a01c69848$06f22e80$0201a8c0@pc1>
Subject: RE: [PHP-DEV] [PATCH] array_fill: Allow an array to specify keys
From: andi@zend.com ("Andi Gutmans")
References: <009a01c69848$06f22e80$0201a8c0@pc1>

I don't have a problem with the functionality and the patch doesn't seem
problematic.
I do question though whether we want to add this to array_fill()? Not sure
if that parameter overloading is very intuitive as it doesn't just add one
parameter. What do others think? Would you prefer an array_fill_keys()?

Andi 

> -----Original Message-----
> From: Matt W [mailto:php_lists@realplain.com] 
> Sent: Sunday, June 25, 2006 4:11 AM
> To: internals@lists.php.net
> Subject: [PHP-DEV] [PATCH] array_fill: Allow an array to specify keys
> 
> Hi all,
> 
> I'm on Windows and haven't tried submitting a patch before, 
> so I JUST now installed WinCvs since I don't think you want 
> just a diff of my local file.
> I used PHP_5_1 since: 5.1's the "regular" download I started 
> working on; if you use it, I assume this can go in 5.1; and I 
> don't know what to do. :-)
> 
> I've been casually browsing through the PHP source for a 
> while, but never modified it or used C.  Just found out about 
> VC++ Express, etc. and set it up to try messing with PHP.  
> Had some missing file errors while building (don't know if 
> that's my problem, or a bug...), but got it done after 
> changing a few files that were causing them.  Just some 
> background... ;-)
> 
> Awhile ago I wished array_fill could also be used like 
> array_fill(array keys, mixed value), which would work like:
> foreach ($keys as $k) { $array[$k] = $val; } OR $array = 
> array_combine($keys, array_fill(0, count($keys), $val));
> 
> So that's what I did for my first, admittedly simple, modification.
> 
> I couldn't figure out a reason for the *newval variable, 
> since it seemed to be the same as **val (pointer stuff can 
> confuse me, being a C newbie ;-)).
> Was I wrong to remove it?  I also wasn't sure why sizeof(val) 
> was used in one place and sizeof(zval *) in another; but it 
> seems to be random after looking in array.c whether var_name 
> or zval * is used, so I stuck with the latter.  I realize now 
> that they're same (right?), but it delayed me, assuming 
> there's a *reason* for everything.
> 
> Other things I changed while experimenting were initializing 
> the return array AFTER checking the parameters so it doesn't 
> have to be destroyed/freed later; and I made the IS_LONG case 
> first in the switch () for the first parameter's type -- 
> isn't that logical since it's supposed to a PHP int and 
> probably is? :-)
> 
> I hope it's coded correctly (feedback welcome, as basic as it 
> is, LOL).
> Everything works like it should as far as I can tell.  And 
> it's a lot faster of course than the other 2 methods I 
> mentioned for initializing arbitrary keys.  Let me know if I 
> need to diff against a version other than 5.1 or whatever.
> 
> 
> Thanks,
> Matt
>