php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Request #70016 SplDoublyLinkedList should return ::key() for key()
Submitted: 2015-07-08 03:49 UTC Modified: 2015-08-26 21:23 UTC
From: me at evertpot dot com Assigned:
Status: Open Package: SPL related
PHP Version: 5.5.26 OS: Any
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: me at evertpot dot com
New email:
PHP Version: OS:

 

 [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

Add a Patch

Pull Requests

Add a Pull Request

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>.
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Sun Aug 09 09:01:24 2020 UTC