php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Request #70016 SplDoublyLinkedList should return ::key() for key()
Submitted: 2015-07-08 03:49 UTC Modified: 2021-07-15 13:57 UTC
From: me at evertpot dot com Assigned: nikic (profile)
Status: Wont fix Package: SPL related
PHP Version: 5.5.26 OS: Any
Private report: No CVE-ID: None
View Developer Edit
 [2015-07-08 03:49 UTC] me at evertpot dot com 
Description:
------------
See code sample =)

Test script:
---------------
<?php

$foo = new SplDoublyLinkedList();
$foo->push('A');
$foo->push('B');

var_dump($foo->key());
var_dump(key($foo));

Expected result:
----------------
int(0)
int(0)

Actual result:
--------------
int(0)
NULL

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-08-18 18:44 UTC] cmb@php.net
-Type: Bug +Type: Feature/Change Request
 [2015-08-18 18:44 UTC] cmb@php.net 
This is not a bug, see the signature of key():

| mixed key ( array &$array )
 [2015-08-18 21:39 UTC] me at evertpot dot com 
current, next and key work on every Traversable. The signature in the docs is simply *also* a bug, albeit a documentation bug.

See here for an alternative example using ArrayAccess:

https://3v4l.org/j0ab9

This has been a PHP feature since the introduction of Iterator.
 [2015-08-18 22:43 UTC] cmb@php.net 
> current, next and key work on every Traversable.

Albeit not necessarily in a meaningful way, see
<https://3v4l.org/4EZrT>.

> The signature in the docs is simply *also* a bug, albeit a
> documentation bug.

Well, I don't think so, because key() internally expects a
HashTable as argument[1]. Passing an arbitrary Traversable might
cause undesirable overhead (and maybe worse; consider generators).

Anyway, as you are insisting that this is a bug, I'm changing back
to bug.

[1] <https://github.com/php/php-src/blob/php-5.6.12/ext/standard/array.c#L928-L937>
 [2015-08-18 22:43 UTC] cmb@php.net
-Type: Feature/Change Request +Type: Bug
 [2015-08-18 23:36 UTC] me at evertpot dot com 
You were right. I was under the assumption that key() mapped consistently to Iterator::key(), but this is not true at all.

In addition, I was also under the assumption that for generators, this syntax:

yield $key => $value

would also map to key(), but this is not true either. I also found out that neither current() nor next() actually give meaningful results on the result of a generator function. That does seem like a bug, as it works for other iterators.

So I'm sorry for the back and forward. I know you guys must get a lot of reports. While I still think that key(), current() and next() should *just* work for any iterator, generator result or array, you were right that at least in the case of key() this was never the case for *most* iterators. So, up to you how you want to treat this.
 [2015-08-26 21:23 UTC] cmb@php.net
-Summary: SplDoublyLinkedList returns NULL for key() +Summary: SplDoublyLinkedList should return ::key() for key() -Type: Bug +Type: Feature/Change Request
 [2015-08-26 21:23 UTC] cmb@php.net 
I still don't think this is a bug, so I'm switching back to
feature request.

I don't deny that it might be nice to extend key(), current() and
next() to work on all Traversables (even though I personally very
rarely use these functions at all), several other array functions
should also be extended the same way for even more consistency,
see also
<https://github.com/php/php-src/pull/1385#issuecomment-121211253>.
 [2021-04-19 12:49 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2021-04-19 12:49 UTC] nikic@php.net 
Assigning to self to track deprecation (https://wiki.php.net/rfc/deprecations_php_8_1#key_current_next_prev_reset_on_objects)
 [2021-07-15 13:57 UTC] nikic@php.net
-Status: Assigned +Status: Wont fix
 [2021-07-15 13:57 UTC] nikic@php.net 
Deprecation has landed, so this is declined.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sat Dec 21 12:01:31 2024 UTC