-
Notifications
You must be signed in to change notification settings - Fork 638
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(collections/unstable): add cycle iterator utility #6386
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6386 +/- ##
=======================================
Coverage 96.23% 96.23%
=======================================
Files 556 557 +1
Lines 42065 42080 +15
Branches 6371 6375 +4
=======================================
+ Hits 40481 40496 +15
Misses 1544 1544
Partials 40 40 ☔ View full report in Codecov by Sentry. |
We have policy to accept new features as unstable features (see https://github.com/denoland/std/blob/main/.github/CONTRIBUTING.md#new-features-in-stable-packages-version--100). Can you prefix the files with I'm not completely sure whether this is aligned with other APIs in |
It seems a bit weird to me for the function to ask for an Iterable when most iterables aren't 'cycle-able'. I think it would make more sense to ask for types that are guaranteed to be cycle-able, like Arrays and Maps. The documentation mentions that if the iterable is empty then the code loops indefinitely. That is a memory leak. The code should check if its empty and either exit or throw an error. export function* cycle<T>(iterable: Iterable<T>): Generator<T> {
let isEmpty = false;
while (true) {
for (const x of iterable) {
isEmpty = false;
yield x;
}
if (isEmpty) break; // Or throw an error as iterable is not cycle-able.
isEmpty = true;
}
} |
let iterator = iterable[Symbol.iterator](); | ||
|
||
while (true) { | ||
const result = iterator.next(); | ||
if (result.done) { | ||
iterator = iterable[Symbol.iterator](); | ||
} else { | ||
yield result.value; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use yield*
this could be much more concise.
let iterator = iterable[Symbol.iterator](); | |
while (true) { | |
const result = iterator.next(); | |
if (result.done) { | |
iterator = iterable[Symbol.iterator](); | |
} else { | |
yield result.value; | |
} | |
} | |
while (true) { | |
yield* iterable; | |
} |
I wonder what some use cases are for these, I've never encountered one myself. |
No description provided.