Skip to content

Conversation

@JosephSilber
Copy link
Contributor

@JosephSilber JosephSilber commented Aug 21, 2019

These methods let you chunk the collection using a callback.

For example, let's say we're trying to process one of Laravel's log files:

LazyCollection::make(function () {
    $handle = fopen('log.txt', 'r');

    while (($line = fgets($handle)) !== false) {
        yield $line;
    }
})
->chunkStartingWith(function ($line) {
    // All Laravel logs start with a timestamp
    // with this format: [2019-08-21 23:54:33]
    return preg_match('^\[\d\{4}-\d\d-\d\d \d\d:\d\d:\d\d\]', $line);
})
->map(function ($lines) {
    return LogEntry::fromLines($lines);
})
->each(function ($logEntry) {
    // Do what you gotta do with $logEntry
});

Using chunkStartingWith, we're returning true wherever we want to start a new chunk; in this case, when the line starts with something like [2019-08-21 23:54:33].

@scrothers
Copy link
Contributor

I personally dislike calling a single item a chunk, a chunk to me feels more like a collection of items.

For example, array_chunk() returns an array of arrays.

Would you be open to calling this itemStartsWith() and itemEndsWith() which I think feels more accurate for what is otherwise an excellent feature it seems.

@devcircus
Copy link
Contributor

If I'm understanding correctly, this doesn't seem to be referring to the item as a "chuck" but instead "the chunk starts with this item".

@JosephSilber
Copy link
Contributor Author

JosephSilber commented Aug 21, 2019

Here are 2 alternatives:

  1. chunksStartingWith() - this is more descriptive, but is no longer a verb like all other methods.

  2. toChunksStartingWith() - while this still isn't a verb, it does feel closer to it 🤷‍♂

I picked chunkStartingWith because it's a verb and fits better with chunk().

Any better ideas?

@BrandonSurowiec
Copy link
Contributor

BrandonSurowiec commented Aug 24, 2019

What if the chunk method was updated to also take a callback? Either specify a number, or let the user decide the chunk splitting logic. Could that work?

Edit: The callback is probably a bit too confusing. I tried some rough code and it takes too much brain power to wrap my head around. Why not name the methods chunkStart and chunkEnd?

collect([1,2,3,4])->chunkStart(function($item) {
    return $item === 2;
}); // [1] [2, 3, 4]

collect([1,2,3,4])->chunkEnd(function($item) {
    return $item === 2;
}); // [1, 2] [3, 4]

@JosephSilber
Copy link
Contributor Author

  1. A callback is extremely counter-intuitive. What's the callback supposed to return, and for what?

  2. chunkStart and chunkEnd aren't very descriptive either.

@donnysim
Copy link
Contributor

Could we pass the current chunk into the callback? for example if we would want to keep the current chunk if the type/name etc. is the same as previous?

warning
warning
error
info
info
error
error

would return chunks:

[warning, warning]
[error]
[info, info]
[error, error]

or maybe we already have something that can achieve that?

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

Sign up for free to 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.

6 participants