Skip to content

Add Nette\Utils\Collection - #127

Open
milo wants to merge 1 commit into
nette:masterfrom
milo:pull-collection
Open

Add Nette\Utils\Collection#127
milo wants to merge 1 commit into
nette:masterfrom
milo:pull-collection

Conversation

@milo

@milomilo commented Jan 9, 2017

Copy link
Copy Markdown
Member
  • bug fix? no
  • new feature? yes
  • BC break? no
  • doc PR: will

I'm using this abstract collection really often. Its purpose is to emulate typed array in most cases.

May seem to be strange that methods like get() or add() are missing. It is because of PHP invariance limitation.

An example of basic usage:

# One item for collectionclass Person
{
public$email;
public$firstName;
public$lastname;
public$username;
}
# Collection itselfclass People extendsNette\Utils\Collection
{
publicfunctionadd(Person$person): People
{
$this->addItem($person, $person->username);
return$this;
}
publicfunctionget(string$username): Person
{
return$this->getItem($username);
}
}
# Creating collection$people = People::fromIterator(...);
# If good friend Joe is here...if ($people->has('joe')) {
debug("Joe is here... again.");
}
# Example usage: create an array of emails, only if email is set$emails = $people->convert(function (Person$person, $key, & $unset) {
$unset = $person->email === NULL;
return$person->email;
});

Collection can declare frequent helpers on self:

# Sorting examplepublicfunctionsortDefault()
{
$this->sortBy(function (Person$a, Person$b) {
if ($a->username === 'joe') return -1; # sorry joereturn$a->username <=> $b->username;
});
}
# Coversion to array for Form <select> inputpublicfunctionforSelectInput()
{
return$this->convert(function (Person$person) {
return"$person->lastname$person->firstName";
});
}

And usage as typed array:

class Mailer
{
publicfunctionmail(People$people, Mail$message)
{
$people->walk(function (Person$person) use ($message) {
$this->senfTo($person->email, $message);
});
}
}

The IteratorAggreagete is here for ordinary loops, like:

/** @var People|Person[] $people */$people = ...;
foreach ($peopleas$person) {
)

The ArrayAccess is not implemented. I tried that, but never found it useful.

The normalizeKey() method can convert complex types to scalar, for example for multi column primary keys in database. On the other hand, I overloaded it very rarely.

If this would be accepted, I'll add tests and doc.

@milomilo changed the title [WIP] Added Nette\Utils\CollectionAdd Nette\Utils\CollectionJan 9, 2017
@Majkl578

Majkl578 commented Jan 9, 2017

Copy link
Copy Markdown
Contributor

From the user's point of view, why would/should one use this, compared e.g. to doctrine/collections (which are even more generic and actually behave like array thanks to ArrayAccess)?

@milo

milo commented Jan 10, 2017

Copy link
Copy Markdown
MemberAuthor

@dg Rebased

@Majkl578 I don't use doctrine collections, but when I take a look at source...

My collections used to look very similarly. Problem arises when you want to be strict on types. You cannot change the get($key) or add($element) signature. So you have to do something like:

publicfunctionadd($element)
{
if (!$elementinstanceof Person) throw...
}

ArrayAccess is a very small part of "behave like array". May seem usable, but consider:

$people[$person->username] = $person;
# vs.$people->add($person);
$people[$person->username]
# vs.$people->get($person->username)
unset($person[$person->username])
# vs.# not implemented, I don't remove items one by one, usually by filter only

And sometimes, IDE has a problem with "napovídání" (mi vypadl anglický termín) with ArrayAccess.

@JanTvrdik

Copy link
Copy Markdown
Contributor

I don't think this belongs to nette/utils.

@milo

milo commented Jan 10, 2017

Copy link
Copy Markdown
MemberAuthor

@JanTvrdik Partially agree. The best would be native typed arrays in the PHP itself.

Collection is a typical part of a model layer, Nette does not have such. This is helper only without big ambitions.

Real world example how I use it:

return People::fromIterator(
$this->dibi->query('...')->setRowClass(Person::class)
);

But not just database. In one project, I'm listing firewall rules from router:

$rules = newFirewall\Rules;
foreach ($this->switch->command(.....) as$line) {
$rules->add(Firewall\Rule::fromCliFormat($line));
}

The point is, that working with typehint People is much more efficient and safe than working with array and @var Person[] annotation.

Btw. there used to be Collections in Nette, but this is different.

@dg
dgforce-pushed the master branch 8 times, most recently from d116328 to 3054b70CompareJanuary 15, 2017 05:55
@milo
miloforce-pushed the pull-collection branch 2 times, most recently from e2486c4 to d9b729cCompareJanuary 16, 2017 11:57
@dg
dgforce-pushed the master branch 2 times, most recently from 496a5dc to 622864eCompareJanuary 16, 2017 12:34
@milo

milo commented Jan 18, 2017

Copy link
Copy Markdown
MemberAuthor

Since its WIP, rebased to some old commit.

@dg
dgforce-pushed the master branch 3 times, most recently from 3897bc7 to b6341f0CompareJanuary 20, 2017 23:14
@milo
milo requested review from dg and removed request for dgMay 3, 2017 15:06
@milo

milo commented May 18, 2017

Copy link
Copy Markdown
MemberAuthor

@dg Do you think it is a good idea to have it in the Utils?

I'm using it literally everywhere and for comfort, I want to have it in a public repo. One possibility is to finish this PR (tests, doc, ...), or to create a milo/collection repo. I'm fine with both options, only want to prevent duplicit work :)

@JanTvrdik

Copy link
Copy Markdown
Contributor

milo/collection is better from my point of view.

@f3l1x

Copy link
Copy Markdown
Member

I like an idea a lot. If it's not suitable for nette/utils I would gladly see it in contributte\utils or also in contributte/collection, something like that was in my plan too.

Please consider that. I could help you @milo with maintenance of course.

Good job. 👍

@dg
dgforce-pushed the master branch 11 times, most recently from 8b993d6 to 98975bfCompareJuly 24, 2017 15:01
@dg
dgforce-pushed the master branch 3 times, most recently from 8aa61b9 to fd48510CompareFebruary 19, 2018 14:44
@dg
dgforce-pushed the master branch 5 times, most recently from e2a373b to a316b52CompareApril 6, 2018 12:09
@dg
dgforce-pushed the master branch 2 times, most recently from 5ac6630 to bc04e9fCompareOctober 18, 2018 23:01
@josefsabl

Copy link
Copy Markdown

It is not Nette related at all, is it? I think that milo/collection would be best.

@milo

milo commented Oct 7, 2019

Copy link
Copy Markdown
MemberAuthor

milo/collection already exists few years :) But as a private repo. Reason to propose for Nette is, that I consider it extremly useful.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@milo@Majkl578@JanTvrdik@f3l1x@josefsabl