Add ArrayCollection map() method capability to access inner array key

[didix16]

This enhancement gives to map() Closure the capability to access also the keys of the inner array holding the elements. Until now, we were able to access just the element on map Closure but not the key. So I propose to add the ability to access also the key for do some statements inside the Closure with key related.

I have the need to add it since in my project I must get the asociative key to take a decision in function of the current key element and I thought that may be usefull for everyone in some other projects to access the key of current processing element

[Ocramius]

Please adjust commit message (to explain what is being done), PR title and tests (required)

[someniatko]

I think you should also update PHPDoc for the Collection interface. Usually doctrine collections are used in context of Doctrine ORM, and it may replace entity's collections with e.g. PersistentCollection. But the user will anyway expect the same behavior for it like for ArrayCollection. Therefore I don't see any sense limiting mapping keys functionality to ArrayCollection only.

[alcaeus]

I don't think we should update the documentation for the Collection interface: not all collection will implement this functionality that way.

[someniatko]

I always thought that ArrayCollection is just an implementation of original Collection interface, and in general it shouldn't add more functionality than declared by the original interface. Because e.g. i have

/** @Entity */
class User
{
    /** @OneToMany(...) */
    private $comments;

    public function __construct()
    {
        $this->comments = new ArrayCollection();
    }

    public function getCommentStats(): Collection
    {
        return $this->comments->map(function ($value, $key) {
             return new CommentStatistics($key, $value);
        }); 
    }
}

I expect that getCommentStats() behaves exactly the same if i've just created new User, and if I fetched this User from a repository. In the latter case the underlying Collection implementation for $comments is no longer ArrayCollection!

[Ocramius]

There's a BC problem here if we move it to Collection. There's a type problem if we don't move it to Collection

Specifically, the signature is currently something like Collection#map(callable (TVal=) : V) : Collection<TKey, V>.

If we change it to Collection#map(callable (TVal=, TKey=) : V) : Collection<TKey, V>, then consumers are still free to ignore the second callable parameter (undefined behavior).

If we don't change it, then Collection#map(callable (TVal=) : V) : Collection<TKey, V> is explicitly incompatible with calling #map with callable (Foo, Bar) : V as parameter, since this callable is of a stricter (type) than what Collection#map() allows.

I don't have a solution for now, but I hope this explains the problem.

[someniatko]

There's a BC problem

This PR targets master so it shouldn't be a problem afaik.

[didix16]

Fixed PHPDoc misstype

[greg0ire]

Please improve your commit message according to the contributing guide.

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

1. git rebase -i origin/master, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
2. A window will show up with many lines, replace pick with fixup on every line but the first one
3. Close your editor, git should do its magic, and you should end up with one commit
4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

[didix16]

Please improve your commit message according to the contributing guide.

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

1. git rebase -i origin/master, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
2. A window will show up with many lines, replace pick with fixup on every line but the first one
3. Close your editor, git should do its magic, and you should end up with one commit
4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

Oh sorry. This is my first PR. I never do it before ( Im very newbie ). I just do it the modifications at Github, not in my local, so I don't know if I can do the rebase at the web page.

What should I do now? I have to clone the repository then?

P.S: For future commits and PR I'll keep in my mind the procedure to make a proper PR.

Thanks in advance

[greg0ire]

I can totally do it for you if you don't feel comfortable jumping through the following hoops:

# Clone the repo
git clone [email protected]:doctrine/collections.git

# Enter it
cd collections

# Add your fork as a remote
git remote add fork [email protected]:didix16/collections.git

# Fetch your patch-1 branch, the overkill way
git fetch --all

# Checkout your branch
git checkout patch-1

# Follow my previous instructions for squashing
git rebase -i origin/master

# Rework your commit message
git commit --amend

# Finally, use the force
git push --force

[didix16]

I can totally do it for you if you don't feel comfortable jumping through the following hoops:

# Clone the repo
git clone [email protected]:doctrine/collections.git

# Enter it
cd collections

# Add your fork as a remote
git remote add fork [email protected]:didix16/collections.git

# Fetch your patch-1 branch, the overkill way
git fetch --all

# Checkout your branch
git checkout patch-1

# Follow my previous instructions for squashing
git rebase -i origin/master

# Rework your commit message
git commit --amend

# Finally, use the force
git push --force

Ok, thank you so much. I'll do it right now and see what happens :) 👍

Needs work

[VincentLanglet]

If we add keys to the map method, maybe the key should be the first param to add consistency with others methods.
See #251

[greg0ire]

This seems stalled, I'm removing it from the 2.0.0 milestone.