Hi,

Why does the Collection interface depend Closure (e.g. for the fillter method) instead of callable?

Just a simple usecase: using classes with __invoke to filter a collection.

(happy to help here)

Followup of #117

What about changing Closure types to callable? For example now I have a problem of passing callable into Collection::map() because static analysers complain about it.

Is there a reason for not adding it?

There appears to be a couple pull requests from 2 years ago that have been merged but never added to a release (#59 & #64) that fixes the very issue I'm having: sorting an ArrayCollection by multiple fields. When can we expect a new release including these fixes? 2 years is kind of a long time to wait for this basic fix.

        $applicableParts = $partCollection->matching(
            Criteria::create()->where(Criteria::expr()->eq('partStatusType', \PartStatusType::Qualified))
        );

The above doesn't work. Below does.

$applicableParts = $partCollection->matching(
            Criteria::create()->where(Criteria::expr()->in('partStatusType', [\PartStatusType::Qualified]))
        );

This seems like odd behavior. Perhaps some documentation would be helpful because on first pass it seems like that one would be able to use the eq() expression with a referenced object. However, that throws the

    {
        return new self(sprintf(
            "Cannot match on %s::%s with a non-object value. Matching objects by id is " .
            "not compatible with matching on an in-memory collection, which compares objects by reference.",
            $class, $associationName
        ));
    }

After playing around with this, I tried out the ->in() expression which seemed to work. Any reason why all matchers couldn't be updated to support the id of the entity itself?

Using API platform.

After the update to 1.6.0 cache:clear throws an error:

Script cache:clear returned with error code 1
!!  
!!   // Clearing the cache for the dev environment with debug                       
!!   // true                                                                        
!!  
!!  
!!  In FileLoader.php line 166:
!!                                                                                 
!!    [Semantical Error] The annotation "@template" in class Doctrine\Common\Coll  
!!    ections\Collection was never imported. Did you maybe forget to add a "use"   
!!    statement for this annotation? in . (which is being imported from "/srv/api  
!!    /config/routes/api_platform.yaml"). Make sure there is a loader supporting   
!!    the "api_platform" type.                                                     
!!                                                                                 
!!  
!!  In AnnotationException.php line 54:
!!                                                                                 
!!    [Semantical Error] The annotation "@template" in class Doctrine\Common\Coll  
!!    ections\Collection was never imported. Did you maybe forget to add a "use"   
!!    statement for this annotation?                                               
!!                                                                                 
!!  

config/routes/api_platform.yml

api_platform:
    resource: .
    type: api_platform

Origin:
@template T in 7ddac10#diff-6ecd3b230c9f7cef1a38ea4da84278f3R27

After manually fixing doctrine/collection in composer.json to 1.5.0 it works again.

I need to define Doctrine Criteria and filter ArrayCollection in the way that every collection's element could be defined with specific field's value as simple scalar or array of scalars (integers). Criteria should define accepted field's value (scalar) and it should be matched if scalar=scalar OR if scalar is in array of scalars.

I have collection of scenarios, like:

$scenarios = new ArrayCollection([
    ['code' => 1],
    ['code' => [2, 3]]
]);

And Criteria like:

$criteria = new Criteria();
$criteria->where($criteria->expr()->contains('code', 1));
$matching = $scenarios->matching($criteria);

But it's not working for arrays since Comparison::CONTAINS only checks if value is part of a field with strpos. Comparison::IN works the other way I need - it checks if collection element field's value is in array specified in criteria's expression.

I've tried to create custom class which implements Doctrine\Common\Collections\Expr\Expression but it ends with "Unknown Expression" in ExpressionVisitor.

Shouldn't ClosureExpressionVisitor::walkComparison() be able to compare it with Comparison::CONTAINS, just with additional check if object's field value is array or not? Use in_array() for arrays or strpos for scalars?

PS. Originally it was question on Stack but after further work I decided to create an issue, because I think it could be helpful if Criteria could support that kind of matching.

I noticed that some methods of Collection return false on failure. Specifically first(), last(), current(), next() and indexOf(). It would be better in my opinion to return null instead.

Since this is a BC break I'd like to propose this for doctrine/collections 2.0.

c3564a6
(Of course all Collection implementations and tests would need to be changed accordingly.)

Hi,
I had a problem with Criteria, which was solved for me on stackoverflow.
The bottom line was an inconsistency between the code in the docs, which I just wanted to forward to you.
Best regards,
Oliver

There are many cases where this would be useful: e.g. in pseudo code

var allUserOrders = user.invoices.flatMap((invoice) => return invoice.orders);

now it's not possible to do that with current Collection API. We need to convert collections to arrays and apply array_reduce, or array_merge. or just use foreach loops and create new collection.

$invoicesOrders = $user->invoices->map(function ($item) { return $item->orders()->toArray(); })->toArray();

$allUserOrders = [];
foreach ($invoicesOrders as $orders) { $allUserOrders = array_merge($allUserOrders, $orders); }

return new ArrayCollection($allUserOrders);

hi,
I update doctrine collections to 1.6 this morning and following exception is throw during my tests :
[Semantical Error] The annotation "@template" in class Doctrine\Common\Collections\AbstractLazyCollection was never imported. Did you maybe forget to add a "use"...

I use doctrine bundle with symfony 4.2, and mapping configuration for my entites use type "annotation"

I think template and template-implements annontations whould be in annotations to ignore on AnnotationReader.
Else do you have any ideas to not have this issue?

what about to add a new feature in the arrayCollection to pluck only property from the object collection list .

example :

class foo {
   protected $id;
   $protected $bar; 
  public function __construct($id , $name)
  {
      $this->id = $id;
      $this->name= $name; 
  }
}
$collection = new ArrayCollection([new foo(1), new foo(2)]);
$propertiesValue = collection->pluck("id");  // [1,2] array or collection 

i already work on this feature , if it seems to be useful , i can work on it .

So I went through the source multiple times and found the method "getObjectFieldValue" to be hard to read, as well as quite inefficient when using the method ArrayCollection::matching(), especially when it comes to string.

I do admit that as a beginner to PHP I did not understand all of it but I do believe myself to have moderate knowledge of it.

Anyways, my solution to this would be the following:

    public static function getObjectFieldValue($object, $field)
    {
        $field = preg_replace_callback('/_(.?)/', function ($matches) {
            return strtoupper($matches[1]);
        }, $field);
        
        if (is_array($object)) {
            return $object[$field];
        }

        $reflection = is_object($object)
            ? new ReflectionObject($object)
            : new ReflectionClass($object);
            

        if ($reflection->implementsInterface(ArrayAccess::class)) {
            return $reflection->getMethod('offsetGet')->invoke($reflection->newInstanceWithoutConstructor(), $field);
        }

        if ($reflection->hasProperty($field)) {
            $property = $reflection->getProperty($field);

            if ($property->isPrivate() || $property->isProtected()) {
                $property->setAccessible(true);
            }

            return $property->getValue($reflection->newInstanceWithoutConstructor());
        }
        
        return static::getMethod($reflection, $reflection->newInstanceWithoutConstructor(), $field);
    }

    protected static function getMethod($reflection, $object, $field)
    {
        foreach (['is', 'get'] as $accessor) {
            $method = sprintf('%s%s', $accessor, ucfirst($field));
            
            if ($reflection->hasMethod($method)) {
                return $reflection->getMethod($method)->invoke($object);
            }
        }
    } 

In case of a non- existent property, assume it to be a method. Did my best to following the old source to the best of my ability. However, I do suggest to allow it to first check the value of an element instead of having it assuming the $object to be an instance of something.

Example

 $c = new ArrayCollection([
     'username' => 'test'
]);

$criteria = new Project\Criteria\Criteria;

var_dump($c->matching($criteria->andWhere($criteria->expr()->contains('username', 'test'))));`

It would then check if the value of the element username matches the requested value, and then in case of match, return just that.

I will once again mention that my proficiency in PHP isn't very high and I'm sure that there are many ways to improve this and would like to have your honest opinion regarding my suggestion.

Friendly regards, Yelson.

Take a look at this

/**
 * {@inheritDoc}
 */
public function remove($key)
{
    if ( ! isset($this->elements[$key]) && ! array_key_exists($key, $this->elements)) {
        return null;
    }
    $removed = $this->elements[$key];
    unset($this->elements[$key]);
    return $removed;
}

this is remove operation of ArrayCollection.

There is also a removeElement operation that, sometimes, is just confused with remove (eg.: you try to remove an Object instead of that object by its id, using remove and not removeElement)

Can't we refactor this name into a more semantic one, like removeElementById?
Personally I find this more clear and less error prone.

We can initially deprecate it to the advantage of the new one and in future, as it will be a BC one, integrate in new major version.

Let me know if I can open a pull request.

The latest version listed in the README.md's Changelog section is v1.6.1. This already had led to confusion when picking the milestone for #210.

Also, as a side question, when is v1.6.3 estimated to be released?

In method ClosureExpressionVisitor::sortByField() we have the following sort function...

    return function ($a, $b) use ($name, $next, $orientation) {
        $aValue = ClosureExpressionVisitor::getObjectFieldValue($a, $name);
        $bValue = ClosureExpressionVisitor::getObjectFieldValue($b, $name);

        if ($aValue === $bValue) {
            return $next($a, $b);
        }

        return (($aValue > $bValue) ? 1 : -1) * $orientation;
    };

... which does not work with Objects, especially DateTime objects. I do not know if it makes sense to compare objects in general if they are greater or lesser, but it definitely makes sense for DateTime objects.

The problem is, that equal DateTime objects will not be recognised by $aValue === $bValue. Instead $aValue > $bValue will evaluate to false in these cases. Not a big deal you may think, as the a element is put below the b element, which is fine, as they are equal. That's right, but $next will neither be executed.

In my use case, sorting by date is the first order criteria, sorting by ID is the second. That code will never evaluate the second criteria, though.

Possible fix:

        if ($aValue instanceof \DateTime && $aValue == $bValue || $aValue === $bValue) {
            return $next($a, $b);
        }

You have to judge whether it makes sense to compare other object types. For DateTimes however, this is necessary, I suppose. Sorting by date with a second order by criteria is not possible otherwise.

Please have a look into the matter.

Update get method with one more additional method parameter which will be used as default value if target element couldn't be found.

Example: Symfony ParameterBag

In my project i create a new object and add it to one of my entity's arraycollection then persist and flush these. Then in the same request i do some more work and want to get back my newly added object with a custom method which didn't worked until i repeated the process.
I realised that the keys of the collection were not the same for the first and the other requests.
When firstly adds an element to the array collection then the keys starts from 0.
After in new requests the keys starts from 1.
I think the indexes should be always the same.

Hi,
I found that if I try to filter over PersistentCollection using contains(there's no LIKE like in query builder) it throws SQL Error cause used key word CONTAINS instead of LIKE in WHERE conditions.

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'CONTAINS '%dsadas%' ...' at line 1

Is BUG or my miss-understand ?

https://github.com/doctrine/collections/blob/master/lib/Doctrine/Common/Collections/ExpressionBuilder.php#L145

This seems somewhat related to #642, so I guess the problem is already known, but anyway: The following code produces unexpected results:

$c = new Doctrine\Common\Collections\ArrayCollection();
$c->add(42);
$c->remove(0);
$c->add(42);
var_dump($c->get(0));

Expected output: 42
Actual output: NULL

Now, one could argue, this is PHP insanity dragged into Doctrine, so work around it yourself. Fair enough:

var_dump(array_values($c->toArray())[0]); // => 42
var_dump($c->getIterator()->current()); // => 42
// Funnily, this fails:
var_dump($c->getIterator()->seek(0)); // => NULL

IMHO, Doctrine should ensure numbering starting from 0 without holes in numeric arrays. But I suspect this won't be done, because the distinction numeric vs. associative is purely context dependent, because the internal array representation is always a hash map?

Hello. I actively use the matching method to select any query results, and this greatly simplifies life. Thank.

But, I noticed that if matching() is the first request, then the doctrine produces an error.

If before matching isEEmpty () or toArray (), then there will be no error.

I suppose that in this case the criterion is not processed, because it is on us that mysql gives an error.

Traces

Doctrine\DBAL\Exception\SyntaxErrorException:
An exception occurred while executing 'SELECT te.id_alcove AS id_alcove, te.type AS type, te.count_messeges AS count_messeges, te.date_updated AS date_updated, te.date_added AS date_added, te.id_resource_post AS id_resource_post, te.id_resource_request AS id_resource_request, te.id_lead_post AS id_lead_post FROM messeges_alcove te JOIN rel_messeges_company t ON t.alcove_id = te.id_alcove WHERE t.company_id = ? AND te. > ?' with params [4, 0]:

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '> '0'' at line 1

  at vendor\doctrine\dbal\lib\Doctrine\DBAL\Driver\AbstractMySQLDriver.php:79
  at Doctrine\DBAL\Driver\AbstractMySQLDriver->convertException('An exception occurred while executing \'SELECT te.id_alcove AS id_alcove, te.type AS type, te.count_messeges AS count_messeges, te.date_updated AS date_updated, te.date_added AS date_added, te.id_resource_post AS id_resource_post, te.id_resource_request AS id_resource_request, te.id_lead_post AS id_lead_post FROM messeges_alcove te JOIN rel_messeges_company t ON t.alcove_id = te.id_alcove WHERE t.company_id = ? AND te. > ?\' with params [4, 0]:SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near \'> \'0\'\' at line 1', object(PDOException))
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\DBALException.php:169)
  at Doctrine\DBAL\DBALException::wrapException(object(Driver), object(PDOException), 'An exception occurred while executing \'SELECT te.id_alcove AS id_alcove, te.type AS type, te.count_messeges AS count_messeges, te.date_updated AS date_updated, te.date_added AS date_added, te.id_resource_post AS id_resource_post, te.id_resource_request AS id_resource_request, te.id_lead_post AS id_lead_post FROM messeges_alcove te JOIN rel_messeges_company t ON t.alcove_id = te.id_alcove WHERE t.company_id = ? AND te. > ?\' with params [4, 0]:SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near \'> \'0\'\' at line 1')
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\DBALException.php:145)
  at Doctrine\DBAL\DBALException::driverExceptionDuringQuery(object(Driver), object(PDOException), 'SELECT te.id_alcove AS id_alcove, te.type AS type, te.count_messeges AS count_messeges, te.date_updated AS date_updated, te.date_added AS date_added, te.id_resource_post AS id_resource_post, te.id_resource_request AS id_resource_request, te.id_lead_post AS id_lead_post FROM messeges_alcove te JOIN rel_messeges_company t ON t.alcove_id = te.id_alcove WHERE t.company_id = ? AND te. > ?', array(4, 0))
     (vendor\doctrine\dbal\lib\Doctrine\DBAL\Connection.php:911)
  at Doctrine\DBAL\Connection->executeQuery('SELECT te.id_alcove AS id_alcove, te.type AS type, te.count_messeges AS count_messeges, te.date_updated AS date_updated, te.date_added AS date_added, te.id_resource_post AS id_resource_post, te.id_resource_request AS id_resource_request, te.id_lead_post AS id_lead_post FROM messeges_alcove te JOIN rel_messeges_company t ON t.alcove_id = te.id_alcove WHERE t.company_id = ? AND te. > ?', array(4, 0))
     (vendor\doctrine\orm\lib\Doctrine\ORM\Persisters\Collection\ManyToManyPersister.php:285)
  at Doctrine\ORM\Persisters\Collection\ManyToManyPersister->loadCriteria(object(PersistentCollection), object(Criteria))
     (vendor\doctrine\orm\lib\Doctrine\ORM\PersistentCollection.php:661)
  at Doctrine\ORM\PersistentCollection->matching(object(Criteria))
     (src\AppBundle\Controller\StatisticsController.php:34)
  at AppBundle\Controller\StatisticsController->statisticAction()
     (vendor\symfony\symfony\src\Symfony\Component\HttpKernel\HttpKernel.php:151)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor\symfony\symfony\src\Symfony\Component\HttpKernel\HttpKernel.php:68)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor\symfony\symfony\src\Symfony\Component\HttpKernel\Kernel.php:200)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (web\app.php:19)

Hi,

After upgrading to the latest version of 1.6.1, the following error is thrown :

[Semantical Error] The annotation "@psalm" in class Doctrine\Common\Collections\AbstractLazyCollection was never imported. Did you maybe forget to add a "use" statement for this annotation?

I suppose the @psalm annotation does exist but in the vimeo/psalm package which is only required in the require-dev section. Any idea how to correct this ?

Rolling back to 1.5.x for the moment.

The documentation for the collection interface lists the matching method along with all other methods even though it isn't included in the same interface but in the Selectable interface. This came up during phpstan/phpstan-doctrine#33.

ExpressionBuilder class has a method isNull() and it's based on equal operator. Why then we can't have a notNull() method based on miscellaneous operator? Is there any problem to compare result with '<>' operator?
If not, my suggestion of method is allegorical for isNull(), and looks like this:

    /**
     * @param string $field
     *
     * @return Comparison
     */
    public function notNull($field)
    {
        return new Comparison($field, Comparison::NEQ, new Value(null));
    }

According to PHP website, iterables are whatever implements Traversable interface. Setting method parameter-types/return-types as iterable helps a lot, but ArrayCollection not implementing that interface, creates a problem.

So my question is: technically being traversable, why Doctrine collections do not implement this interface, to make them "legally" iterable?

Thanks.

In this file:

https://github.com/doctrine/collections/blob/master/lib/Doctrine/Common/Collections/ArrayCollection.php#L341

    public function filter(Closure $p) : Collection
    {
        return $this->createFrom(array_filter($this->elements, $p, ARRAY_FILTER_USE_BOTH));
    }

When you filter the array collection the php array keys change.

Original array
{
[0] =>
[1] =>
[2] =>
}

When you filter the array and filter the 1 element, this is the result

Original array
{
[0] =>
[2] =>
}

Is possible change this behavior?

We can add array_values function

Change
return $this->createFrom(array_filter($this->elements, $p, ARRAY_FILTER_USE_BOTH));

by

    return $this->createFrom(array_values(array_filter($this->elements, $p, ARRAY_FILTER_USE_BOTH)));

Thanks.

The Data Structures (DS) extension comes with an awesome interface and killing performance. Why not to use it? There's even a polyfill for when the client doesn't want to add a new extension.

See question here where this was originally requested #125 (comment)

It would be quite nice t have CONTRIBUTING.md at the root of current repository. That would eliminate efforts to find commit check-list, codestyle guides, etc.

Given the presence of map and filter, not having just reduce goes sort of against common sense and basic assumptions. We already have forAll and exists which are specific cases of reduce. Why not also have the general-purpose higher order function? Heck, it's kind of the highest higher-order function.

When I updated doctrine/collections from 1.5.0 to 1.6.0 I have this error [Semantical Error] The annotation "@template" in class Doctrine\Common\Collections\ArrayCollection was never imported. Did you maybe forget to add a "use" statement for this annotation?

How can we fix it ?

I have some collections subclassing ArrayCollection and doing thing like this:

class EpisodeCollection extends ArrayCollection
{
    public function getInPeriod(Period $period): self
    {
        return $this->filter(static function (EpisodeInterface $episode) use ($period): bool {
            return $period->contains($episode->getDate());
        });
    }

From my perspective this works fine, I want to return an instance of my own collection from its method and that's how upstream filter() does work when subclassed (down to createFrom() returning new static).

However this gets Psalm all bothered:

ERROR: MoreSpecificReturnType - src\Tv\Collection\EpisodeCollection.php:12:50 - The declared return type 'Rarst\Tv\Collection\EpisodeCollection' for Rarst\Tv\Collection\EpisodeCollection::getInPeriod is more specific than the inferred return type 'Doctrine\Common\Collections\ArrayCollection<array-key, mixed>'
    public function getInPeriod(Period $period): self


ERROR: LessSpecificReturnStatement - src\Tv\Collection\EpisodeCollection.php:14:16 - The type 'Doctrine\Common\Collections\ArrayCollection<array-key, mixed>' is more general than the declared return type 'Rarst\Tv\Collection\EpisodeCollection' for Rarst\Tv\Collection\EpisodeCollection::getInPeriod
        return $this->filter(function (EpisodeInterface $episode) use ($period): bool {
            return $period->contains($episode->getDate());
        });

I am guessing this is over upstream methods being @psalm-return ArrayCollection<TKey,T> ?

Am I missing something here on how I am (not) supposed to subclass and/or document this?

Don't use hyphens

• @template TKey of array-key

composer req vimeo/psalm

php vendor/bin/psalm --show-info=false

Scanning files...

Fatal error: Uncaught Psalm\Exception\TypeParseTreeException: no hyphens allowed in /var/www/stf/vendor/vimeo/psalm/src/Psalm/Type/Atomic.php:129
Stack trace:
#0 /var/www/stf/vendor/vimeo/psalm/src/Psalm/Type.php(117): Psalm\Type\Atomic::create('Doctrine\\Common...', false, Array)
#1 /var/www/stf/vendor/vimeo/psalm/src/Psalm/Visitor/DependencyFinderVisitor.php(661): Psalm\Type::parseTokens(Array)
#2 /var/www/stf/vendor/vimeo/psalm/src/Psalm/Visitor/DependencyFinderVisitor.php(200): Psalm\Visitor\DependencyFinderVisitor->registerClassLike(Object(PhpParser\Node\Stmt\Interface_))
#3 /var/www/stf/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(200): Psalm\Visitor\DependencyFinderVisitor->enterNode(Object(PhpParser\Node\Stmt\Interface_))
#4 /var/www/stf/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray(Array)
#5 /var/www/stf/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Namespace_))
#6 /va in /var/www/stf/vendor/vimeo/psalm/src/Psalm/Type/Atomic.php on line 129

Scenario
I want be able to filter an array by key or both (key, value)

    /// DOESN"T WORK :(
    (new ArrayCollection(['italian' => 'pizza', 'turkish' => 'kebab']))
        ->filter(function($k, $v) {
            return 'turkish' === $k;
        })
    // WORKSS!1!!  
    array_filter(['italian' => 'pizza', 'turkish' => 'kebab'], function($k, $v) {
            return 'turkish' === $k;
        }, ARRAY_FILTER_USE_BOTH);

Suggested Solution
Since PHP 5.6 we are able to specify a flag on array_filter which allows to filter the element of the array by passing to the closure that filter the array, key, value or both

array array_filter ( array $array [, callable $callback [, int $flag = 0 ]] )

Flag determining what arguments are sent to callback:

ARRAY_FILTER_USE_KEY - pass key as the only argument to callback instead of the value
ARRAY_FILTER_USE_BOTH - pass both value and key as arguments to callback instead of the value

Default is 0 which will pass value as the only argument to callback instead.

Suggested Implementation
At the moment ArrayCollection#filter uses the default value 0 which makes the flag feature impossible to use.

-    public function filter(Closure $p)
+    public function filter(Closure $p, int $flag = 0)
    {
-        return $this->createFrom(array_filter($this->elements, $p));
+        return $this->createFrom(array_filter($this->elements, $p, $flag));
    }

Concerns
I understand that exposing lower level APIs to an higher level of abstraction is not probably a good idea. Yet, the additional feature would be useful overall to the client who uses the method

If you provide some feedback I can submit a PR

Jira issue originally created by user @Ocramius:

The current collection API is very flexible, but incomplete and by far respecting naming in the domain of collections/maps/arrays/stacks/lists/etc.

We may want to look at something like https://github.com/morrisonlevi/Ardent as a replacement.

Jira issue originally created by user anton.serdyuk:

Given I have some entity method like this:

    public function hasNewComments()
    {
        $newComments = $this->comments->matching(
            Criteria::create()
                ->where(Criteria::expr()->eq('new', true))
        );

        return count($newComments) > 0;
    }

Where $this->comments is OneToMany collection.

If $this->comments is EXTRA_LAZY, then this code will always produce SQL query to database and always will work fine. But if it is not EXTRA_LAZY and appears to be loaded before (for eample count($this->comments) was called before), then this code will produce error "Trying to get private property Comment::$new from ClosureExpressionVisitor context".

I thins we should have something like reflection or \Closure::bind() stuff to get actual field value within ClosureExpressionVisitor regardless this field is private or public.

I know I can create getter getNew() or isNew(), but I believe this is known to be a good OOP practice to avoid unnecessary getters, and I am not sure collection api should make this decision for me.

I don't know how many other people are in need of it, but dealing with a a relational DB and the Doctrine Discriminator mapping is really handy. That is, until you need to limit the results from a Collection to a relation that has a relation of a particular discriminator type.

For example:

/**
 * @ORM\Entity()
 */
class ParentObject
{
    /**
     * @var Collection
     * @ORM\OneToMany(targetEntity="ChildObject", mappedBy="parentObject")
     */
    private $childObjects;

    public function getPublishedChildObjects()
    {
        $criteria = Criteria::create();
        $criteria->andWhere(
            Criteria::expr()->instanceOf('childObjects.status', Status\Published::class);
        );
        return $this->childObjects->matching($criteria);
    }
}

/**
 * @ORM\Entity()
 */
class ChildObject
{
    /**
     * @var ParentObject
     * @ORM\ManyToOne(targetEntity="ParentObject", inversedBy="childObject")
     */
    private $parentObject;

    /**
     * @var Status
     * @ORM\ManyToOne(targetEntity="Status")
     */
    private $status;
}

/**
 * @ORM\Entity()
 * @ORM\InheritanceType("SINGLE_TABLE")
 * @ORM\DiscriminatorColumn(name="type", type="string")
 * @ORM\DiscriminatorMap({
 *     "draft" = "Status\Draft",
 *     "published" = "Status\Published"
 * })
 */
class Status
{
    private $id;
}

/* @var ParentObject $parentObject */
$parentObject = <Get ParentObject from query>;
$publishedChildObjects = $object1->getPublishedChildObjects();

The actual need is more complex with multiple status types. It would be nice to be able to do the above with collections.

I've run into a situation where I'm using ArrayCollection to filter some objects using Criteria::orderBy().

Assume these objects have a private property 'foo', and a public method 'foo()' which returns the private property value. The ClosureExpressionVisitor will check for various public methods existence (for example is*/get* variants, or access through magic methods like __get/__call).

I have the feeling there is one more check missing here, which is checking if a public method exists that matches the field name literally (without prepending get/is or through magic methods).

I'm not sure if this was omitted with a reason, maybe this means we are using a convention thats not very widespread? I will create a PR to hopefully communicate my issue better.

Similar to #200 but about map() method.

I have created a reproduction repository: https://github.com/someniatko/doctrine-collections-map-psalm

doctrine/collections: v1.6.2
vimeo/psalm: 3.6.2

We are trying to filter an entity relation with a Doctrine Criteria

    /**
     * @ORM\OneToMany(targetEntity="Message")
     */
    private $messages;

    public function getMessagesWithCriteria() {
        $criteria = new Criteria();
        $criteria->where(Criteria::expr()->eq('field', 'value'));
        return $this->messages->matching($criteria);
    }

In a controller when you call first the original relation getMessages() and then the filtered relation getMessagesWithCriteria() like this :

$mainEntity->getMessages()->last()->getId();
$mainEntity->getMessagesWithCriteria()->last()->getId();

The ClosureExpressionVisitor is causing an exception by calling the property with a bad accessor "getfield()" instead of "getField()"
The ClosureExpressionVisitor is not camel casing the field name line 58 :

$accessor = $accessors[0] . $field;

Is the parameter $field in ClosureExpressionVisitor::getObjectFieldValue($object, $field) must be already in camelcase or in lowercase ?

Considering this code:

<?php

use Doctrine\Common\Collections\ArrayCollection;

$extendedCollection = new class ([]) extends ArrayCollection {};
$extendedCollection->filter(fn () => true);

Since this change in psalm released in version 3.6.6:
vimeo/psalm#2326

When analyzing the code above with psalm I get the following error:

ERROR: TooManyTemplateParams - src/Application/test.php:6:1 - _var_www_src_Application_test_php_5_87 has too many template params, expecting 0
$extendedCollection->filter(fn () => true)

Bug Report

Summary

ArrayCollection class has unitialised private $elements property which leads to null value in it in some cases.
It needs to be initialized:
private $elements; => private $elements = [];

Current behavior

In case of deserialization the value of the property is not safty - it might be null! As the result - I getting error exception when calling ArrayCollection's methods which contains inner calls to php functions with array only arguments (e.g. getValues() calls array_values()).

How to reproduce

1. Extend ArrayCollection, and implement /Serializable interface (add custom seialize and userialize methods).

class ExtendedArrayCollectionWithCustomSerializer extends ArrayCollection implements Serializable
{
    public function serialize()
    {
        return json_encode($this->getKeys());
    }

    public function unserialize($serialized)
    {
        // $this->clear(); // Workaround to force $elements initialisation

        foreach (json_decode($serialized) as $value) {
            parent::add($value);
        }
    }
}

2. Create an empty ArrayCollection instance, then serialize and userialize it, then check unserialized instance - it will have null value in $elements property.
3. Call something like getValues() you will get an error: Warning: array_values() expects parameter 1 to be array, null given

$collection = new ExtendedArrayCollectionWithCustomSerializer();
$collection = unserialize(serialize($collection));
// At this point $collection->elements === null
$collection->getValues();  // ErrorException there: Warning: array_values() expects parameter 1 to be array, null given

Expected behavior

Have $elements property either protected or initilized so we can write safety deserialization code (and therefore never get null as $elements property value)

While reading about using collections to improve a code base, I learned about the difference between a map() and a reduce() function.

While this library currently has the map() function, it seems to 'miss' a reduce() function. This would be nice to, for example, calculate the total price of a collection of OrderLines.

Inspiration here. Would a PR be considered? Or is this library not intended to become bloated / complete? Just wondering 😄

Hi everyone !

I am facing a bug using ArrayCollection->filter(). As it use array_filter internally, the elements attribute of the collection is an associative array after the filter.
Because array_filter removes elements without reindexing keys, we should use array_values to avoid serialization problem (due to associative array, instead of pure array).

The fix is relatively easy to apply and shouldn't create any problem.

public function filter(Closure $p)
{
    return $this->createFrom(array_values(array_filter($this->elements, $p)));
}

What did you think about ?

Hi!

I found not valid behavior for ArrayCollection matching function.

$criteria = Criteria::create()
                ->where(Criteria::expr()->eq("date", $date));

$result = $this->meta->matching($criteria);

For ArrayCollection matching use code from

# Doctrine/Common/Collections/Expr/ClosureExpressionVisitor.php (lines 112 - 116)
switch ($comparison->getOperator()) {
   case Comparison::EQ:
      return function ($object) use ($field, $value) {
            return ClosureExpressionVisitor::getObjectFieldValue($object, $field) === $value;
}

But

$date1 = new DateTime('2016-09-05');
$date2 = new DateTime('2016-09-05');
$result = $date1 === $date2; ///false

$result = $date1 !== $date2; ///true

Need fix this.

In method ClosureExpressionVisitor::sortByField() we have the following sort function...

    return function ($a, $b) use ($name, $next, $orientation) {
        $aValue = ClosureExpressionVisitor::getObjectFieldValue($a, $name);
        $bValue = ClosureExpressionVisitor::getObjectFieldValue($b, $name);

        if ($aValue === $bValue) {
            return $next($a, $b);
        }

        return (($aValue > $bValue) ? 1 : -1) * $orientation;
    };

... which does not work with Objects, especially DateTime objects. I do not know if it makes sense to compare objects in general if they are greater or lesser, but it definitely makes sense for DateTime objects.

The problem is, that equal DateTime objects will not be recognised by $aValue === $bValue. Instead $aValue > $bValue will evaluate to false in these cases. Not a big deal you may think, as the a element is put below the b element, which is fine, as they are equal. That's right, but $next will neither be executed.

In my use case, sorting by date is the first order criteria, sorting by ID is the second. That code will never evaluate the second criteria, though.

Possible fix:

        if ($aValue instanceof \DateTime && $aValue == $bValue || $aValue === $bValue) {
            return $next($a, $b);
        }

You have to judge whether it makes sense to compare other object types. For DateTimes however, this is necessary, I suppose. Sorting by date with a second order by criteria is not possible otherwise.

Please have a look into the matter.

See https://github.com/doctrine/doctrine2/issues/6712 for original issue and PR demonstrating it.

I think there may have been a mistaken release of V1.4 on 2017-01-03 by @mikeSimonson

• The tagged version is not conform with the previous version
• There are no release notes
• Changes from over a year (v1.3.0 was released in 2015-04-15

Could you at least write some release notes, please? This library is used by a ton of people..

I'm wondering if there was any specific reason to go with a single "Collection" class instead of implementing more sophisticated sub-classes like:

• Maps: Elements are stored by key (currently Collection::set())
• Lists: Elements are simply appended (currently Collection::add())
• Sets: Like lists, but guarantees no duplicate elements (currently no equivalent)