Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first util to get the first item from the generator #4938

Closed
pjasiun opened this issue Mar 14, 2017 · 9 comments · Fixed by ckeditor/ckeditor5-utils#135
Closed

first util to get the first item from the generator #4938

pjasiun opened this issue Mar 14, 2017 · 9 comments · Fixed by ckeditor/ckeditor5-utils#135
Assignees
Labels
package:utils type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Milestone

Comments

@pjasiun
Copy link

pjasiun commented Mar 14, 2017

It's basically generator.next().value;, but:

first( doc.selection.getSelectedBlocks() );

seems to be more readlable them:

doc.selection.getSelectedBlocks().next().value;

I know we have nth, but it's a common case when you wants to get the first value and we could have a dedicated method for it.

@Reinmar
Copy link
Member

Reinmar commented Mar 14, 2017

We've got nth() already, but I also see that first() is being the most popular use case in the normal code. nth() is mostly used in the tests.

@ma2ciek
Copy link
Contributor

ma2ciek commented Mar 15, 2017

But similarly to nth() implementation the implementation of first() shouldn't change the iterator, right? (Like iterator.next().value does)

@pjasiun
Copy link
Author

pjasiun commented Mar 15, 2017

What do you mean? As far as I know nth() do change the iterator, but you can check it to be sure.

@ma2ciek ma2ciek self-assigned this Mar 15, 2017
@ma2ciek
Copy link
Contributor

ma2ciek commented Mar 15, 2017

I checked it and the nth( index, iterable ) function even finishes the iterator generator no matter what the index is, because every for of loop finishes the generator. So maybe the first( iterator ) function should run inside just nth( 0, iterator ) :D

@Reinmar
Copy link
Member

Reinmar commented Mar 15, 2017

Interesting... I'd love first() to work like shift() because subsequent first() calls may be a usecase one day. But nth() can read the entire iterable. It's even better if nth() reads the entire iterable, because then you'd have a better chance to notice that something went wrong (if you didn't know that it consumes the items)

@ma2ciek
Copy link
Contributor

ma2ciek commented Mar 15, 2017

Ok. So the PR for the first() is ready and the missing parts are fixed. It works similar to the shift() for now. I'll report an issue for the nth() function improvement.

@Reinmar
Copy link
Member

Reinmar commented Mar 15, 2017

As I commented in ckeditor/ckeditor5-utils#130 I find my previous proposal totally wrong now. I take it back :P

@pjasiun
Copy link
Author

pjasiun commented Mar 16, 2017

If we want to pop the first value multiple times maybe we should have a better name, like pop or head (note that lodash use head and first is the alias for it https://lodash.com/docs/4.17.4#head)?

@Reinmar
Copy link
Member

Reinmar commented Mar 16, 2017

pop() means taking the last item. And I like first() more than head(). But I'm fine with both.

pjasiun referenced this issue in ckeditor/ckeditor5-utils Mar 16, 2017
@mlewand mlewand transferred this issue from ckeditor/ckeditor5-utils Oct 9, 2019
@mlewand mlewand added this to the iteration 9 milestone Oct 9, 2019
@mlewand mlewand added status:confirmed type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:utils labels Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:utils type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants