HashMap toArray() does not use $preserveKeys by default

[Mikulas]

While $preserveKeys = FALSE is default everywhere, wouldn't it be better to break the consistency for Maps? Or better yet, preserve keys by default? It feels like it should be more general use case.

From the user standpoint, I would rather use self describing $map->toArray(). The additional boolean argument is not self explanatory (on first sight, without checking the function declaration). And while $map->toArray($withKeys = TRUE); is readable, it does not really cut the mustard as it's waaaay too long.

[morrisonlevi]

I am actually unsure what to do about keys in general. There is a ticket open about preserving keys on slice and this falls into a similar vein of thought. My current thinking is to not preserve keys by default at all and possibly remove the options to preserve keys. The reasoning is that out of the whole library, Maps are the only ones that even care about keys at all. In fact, the concept of having a key in many of them doesn't make sense by definition, such as in a Set. I do agree that Maps specifically would benefit from this key preservation in many of the methods but I am currently searching for a nice solution to both issues.

I'd like to hear more arguments on both sides of this issue.

[ghost]

Just thinking out loud here,

I would think this is difficult to get right, due to the manner in which PHP handles arrays. Generally speaking, I consider the following:

• I see a distinction between "indices" and "keys"; indices always exist, keys are implementation dependent.
• The numeric index of any single-dimensional collection should be the 0-based offset in the collection at which a given element resides.
• Whether or not the numeric index is relevant to general use-cases is dependent on the implementation (OrderedList, Dictionary, HashMap, etc.)

For instance, a Dictionary implementation may actually be a collection of Pair objects. The index of a given Pair is the index of the underlying array at which a given Pair resides.

<?php
$dictionary->getElementAt(7);  // yields the element at index 7 in the underlying collection
$dictionary->getElement($key); // performs an implementation specific lookup for $key

Depending on the implementation, the order of which the elements are stored in the underlying collection may not be relevant, and getElementAt() becomes all but useless.

Why does this translate to the problem at hand?

I would argue that the keys generated from iterator_to_array should always be the numeric indices of the collection. Unfortunately, that means that the values now become responsible for holding the keys if the implementation has them.

<?php
var_dump($hashMap->toArray());

Would possibly yield:

array(3) {
  [0] =>
  array(1) {
    'acbd18db4cc2f85cedef654fccc4a4d8' =>
    string(3) "foo"
  }
  [1] =>
  array(1) {
    '37b51d194a7513e45b56f6524f2d51f2' =>
    string(3) "bar"
  }
  [2] =>
  array(1) {
    'd85b1213473c2fd7c2045020a6b9c62b' =>
    string(3) "qux"
  }
}

[Mikulas]

I'm afraid I don't really see the big picture, but I would rather work with a easily usable and readable API than work with a flawless theoretical design. I do understand the rationalization behind your last example, but without extensive syntactic sugar (not really doable in php), I don't even want to imaging working with such toArray() result.

Then again I might be misusing this library. What I understand any STL should do is create semantic data types which when used form more readable code. Also, hinting is somewhat compelling reason: having a standard php array used as hash map does not limit use of e.g. array_reverse, even though it makes no sense in the context. HashMap class is limited only to methods that make sense for the type.

3rd party libraries always take raw array as argument, which is reasonable. I want to work with abstract data structures in my own code though, so what I would run into is for example a HashMap $map and function foo(array $arg) interface that expects [key => value] format. I can't really imagine anything other than foo($map->toArray()) and foo((array) $map). Now of course the library might expect a different format of array, but this one is arguably the most used one. It just feels intuitive hash map to array would return keys. Again I don't really understand the profound theory behind all this, I'm just trying to imagine what library I want to use.

[ghost]

Believe me, I am no authority on flawless theoretical design (or even design in general) My comment was more of an on-topic stream of conscious ;-)

Ultimately, due to the fact that Ardent hasn't (yet) become a dependency to many libraries, the likelihood is that you'll have to map the type into a native array of . Hopefully, libraries like (but preferably only) Ardent will become ubiquitous, and HashMap et al., will become a common argument type.

[morrisonlevi]

I think the reason we are in this state of things is because in PHP arrays always have keys and more critically here, so do Iterators. So naturally I preserved the $preserveKeys nature of toArray.

I am still undecided on what to do, but I have some changes that will be coming tomorrow (yay BC Break Thursday!) that will cause toArray to always use the keys provided and expose two new methods keys() and values() that do pretty much what you expect them to do. I think I like these changes overall and will think about them tonight and tomorrow while in the shower (best thinking time of the day).

[morrisonlevi]

It's been a while and I like the idea of preserving keys whenever possible. The reason is that if you strip keys you can't get them back. If you don't want the keys you can always strip them. I should review all the structures and algorithms to do this whenever possible.

[hakre]

The one function that comes to mind for me on this topic from PHP core is iterator_to_array. It has a preserve keys parameter ($use_keys) which is optional and defaults to true.

Only in cases where I use it and where I know the keys are standing in the way I can switch it to off by setting it.

I think this follows the same principle as outlined by @morrisonlevi in the last comment.

So perhaps this is already fixed by switching to such a default?