Skip to content

Strange behavior with Seq range, reverse #1730

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

Closed
algoshipda opened this issue Aug 29, 2019 · 3 comments · Fixed by #1967
Closed

Strange behavior with Seq range, reverse #1730

algoshipda opened this issue Aug 29, 2019 · 3 comments · Fixed by #1967
Milestone

Comments

@algoshipda
Copy link

algoshipda commented Aug 29, 2019

What happened

Seq([1, 2, 3]).zip(Range())
  .reverse()
// expected: [[3, 2], [2, 1], [1, 0]]
// actual: [[3, Infinity], [2, Infinity], [1, Infinity]]

and run this code below with https://clojurescript.io/

(reverse (map vector [1 2 3] (range)))
; produce ([3 2] [2 1] [1 0])

I think those codes are equivalent and Clojure's output is correct output.
Is this desired behavior?

Immutablejs Version: 4.0.0-rc.12

@jdeniau
Copy link
Member

jdeniau commented Jul 19, 2021

It might be a side-effect of Range function defaulting to Infinity. You can see #1801 for more informations.
I think this will be fixed once we decide an implementation on #1801.

You can force the behaviour as a matter of fact:

Seq([1, 2, 3]).zip(Range(0, 3))
  .reverse()
// results as expected: : [[3, 2], [2, 1], [1, 0]]

@jdeniau
Copy link
Member

jdeniau commented Aug 31, 2023

OK I think I do understand the issue:

actions are deferred to the latest moment possible, to [0, 1, 2] is not generated before the reverse call.
When you call reverse, then we calculate like that:

  • take the latest value of the Range,
  • iterate from that number and decrement 1 by 1,

But as the "end" value is Infinity, then it will take Infinity, Infinity - 1 (resolved as Infinity) and Infinity - 2 (same here).

#1967 does change the Range function and will force you to have a start and end value, so I will not fix this as you will forced to do so in your code (unless you specify Infinity yourself, but then that's up to you 😅 )

In a probably more complex example, you will need to do that:

const mySequence: Array<number> = getSequence();

Seq(mySequence).zip(Range(0, mySequence.length - 1)).reverse();

You can change the reverse position if you think that it's more readable:

const mySequence: Array<number> = getSequence();

Seq(mySequence)reverse().zip(Range(mySequence.length - 1, 0));

@jdeniau
Copy link
Member

jdeniau commented Aug 31, 2023

Closing as you should explicitly set start and end values in your range in 4.x version

@jdeniau jdeniau closed this as completed Aug 31, 2023
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 a pull request may close this issue.

2 participants