Documentation: Collection Principles #174
Conversation
|
@drupol hey! There are a few more things which I want to add here, but feel free to have a read of what's there so far and let me know what you think 😁 |
|
Heya! I'll make a global feedback at the end, but from what I've seen so far, it's great ! Have a great night |
docs/pages/principles.rst
Outdated
|
|
||
| Sometimes you might want your ``Collection`` object to behave eagerly. | ||
| This is a legitimate need when you want to trigger side-effects, like persisting | ||
| entities to a database or sending a batch of emails. |
There was a problem hiding this comment.
haha ! People I work with are using Collection for doing things like that. As I'm doing the code reviews most of the time, I always ask them to use a regular foreach loop instead.
Why?
Because I ultimately think that using Collection for doing tasks with side-effects is not the proper way to use this tool.
According to me, if you use Collection without using it's return value, then you're using it wrong.
I'm definitely aware that technically it will work, but semantically, it's worse than using a plain old foreach (or alike) loop.
There was a problem hiding this comment.
hmm I get what you mean, I guess I'm just not a big fan of loops in general 😅 .
I see Collection::apply as a replacement for a separate foreach loop (ofc you have to use squash as well), because to me doing apply + foreach seems redundant.
In Rust, iterators have a for_each method which basically acts like apply but is not lazy, which means it can be a substitute for a loop when you want to do some side effects, especially at the end of a chain of other transformations: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=3bcb45d0c7f99d3a91e98e65cf87c371
There was a problem hiding this comment.
This being said, I'll add a note that normal iteration via a foreach loop should be preferred for triggering side-effects
There was a problem hiding this comment.
Ok I see, thanks for the Rust example.
Maybe add a note in this direction:
Collection is a helper for making transformations to input data. Despite the fact that you can technically do side-effects in some operations(due to the fact that you can pass a custom Closure that can alter variables out of its scope) , it's better to avoid using the library for such endeavor, but rather to get the return value of a transformation.
There was a problem hiding this comment.
There was a problem hiding this comment.
There's no conflict, no worries. Apply has been made for that, but I don't advise to use it at all, that's it :)
|
I'm wondering if we should have a word about performance. WDYT ? I also updated the upcoming changelog and added a sentence about this new documentation page. |
What are you thinking to mention regarding performace? |
That sometimes it might be slower using |
Hmm I'm not sure if this is the best place for this I think. But this gave me an idea for a separate section I can add in the future, where this would be a great mention. This section would compare how to do the same thing:
|
| $this::fromIterable([$a]) | ||
| Collection::fromIterable([$a]) |
There was a problem hiding this comment.
bad copy-paste here 😅
|
@drupol I added the remaining sections I wanted now. Have a read through and let me know if it makes sense and any feedback you might have 😁 |
docs/pages/principles.rst
Outdated
| ->contains('d'); // false | ||
|
|
||
| $result = Collection::fromIterable(range('a', 'c')) | ||
| ->contains('a', 'z'); // true |
There was a problem hiding this comment.
How about adding an example with a AND and OR using filter?
There was a problem hiding this comment.
in addition to this or instead of this example?
|
@drupol anything else on this? Otherwise it's ready from my side 🚀 |
This PR adds a new section in the documentation to describe a few principles that are at the core of this package.
Also:
Related to #106 (comment).