Return Listing in getChildren methods

[passioneight]

The line $this->children[$cacheKey] = $list->load(); loads the complete list. Seems like this should be done only when needed, not when getChildren is called.

Returning the Listing instead would further allow for more flexibility; one could:

• alter the Listings conditions or
• sort the result or
• limit the result (e.g. for pagination)
• ...

Note that this is only one of the many getChildren methods. Changing this is not trivial, as one would probably expect all of these methods to return the same data type. It's also a potential BC for any application calling getChildren if they access the data directly - as it is done here.

---

Currently implemented code from the link above:

/**
 * Get a list of the children (not recursivly)
 *
 * @param bool $includingUnpublished
 *
 * @return self[]
 */
public function getChildren($includingUnpublished = false)
{
    $cacheKey = $this->getListingCacheKey(func_get_args());

    if (!isset($this->children[$cacheKey])) {
        $list = new Document\Listing();
        $list->setUnpublished($includingUnpublished);
        $list->setCondition('parentId = ?', $this->getId());
        $list->setOrderKey('index');
        $list->setOrder('asc');
        $this->children[$cacheKey] = $list->load();
    }

    return $this->children[$cacheKey];
}

Suggested changes:

/**
 * Get a list of the children (not recursivly)
 *
 * @param bool $includingUnpublished
 *
 * @return Document\Listing
 */
public function getChildren($includingUnpublished = false)
{
    $list = new Document\Listing();
    $list->setUnpublished($includingUnpublished);
    $list->setCondition('parentId = ?', $this->getId());
    $list->setOrderKey('index');
    $list->setOrder('asc');

    return $list;
}

Note that the suggested changes do not consider any kind of caching for brevity.

---

What's your take on this?

[dvesh3]

@passioneight how about we add a default param? something like $loadList = true. you can pass false to return list instead of array. this way it is backward compatible.

[passioneight]

At the time of writing, I was kinda hoping you'd change it in Pimcore X, as a BC wouldn't have mattered as much.

Not sure about the parameter: in the long run, the list shouldn't be loaded by default - which probably means the suggested parameter will be removed again, resulting in a BC, just a bit later.

Additionally, the parameter needs to be appended (i.e., added after $includingUnpublished), which means that I'd always have to call getChildren(false, true), which - subjectively speaking - isn't exactly a solution, but a workaround.

So, I'd prefer to see this change as BC in Pimcore X, as it is still in beta. Any chance we kind change it there?

[passioneight]

@dvesh3 maybe you could implement this for v11?

[dvesh3]

Please create an issue. thanks!

[passioneight]

#11737