Provide Collections#shuffle as extensions for collections in JS #1300
Conversation
| } | ||
| } | ||
| } | ||
| private fun rand(upperBound: Int) = Math.floor(Math.random() * upperBound) |
There was a problem hiding this comment.
Couldn't find any way to make Random customizable in JavaScript, therefore that variant is left out.
There was a problem hiding this comment.
We're going to provide common Random , see https://youtrack.jetbrains.com/issue/KT-17261, though it isn't there yet.
| } | ||
| } | ||
| } | ||
| private fun rand(upperBound: Int) = Math.floor(Math.random() * upperBound) |
There was a problem hiding this comment.
This requires importing kotlin.js.Math class.
There was a problem hiding this comment.
Thanks, done.
Not sure how to test this exactly, any help would be appreciated! :)
| public fun <T> MutableList<T>.shuffle(): Unit { | ||
| (lastIndex downTo 1).forEach { i -> | ||
| rand(i + 1).let { j -> | ||
| swap(i, j) |
There was a problem hiding this comment.
Fixed with a private method instead, thanks!
| */ | ||
| @SinceKotlin("1.2") | ||
| public fun <T> MutableList<T>.shuffle(): Unit { | ||
| (lastIndex downTo 1).forEach { i -> |
There was a problem hiding this comment.
JS backend doesn't optimize such loops, so it would be better to rewrite it as plain old while loop.
There was a problem hiding this comment.
for (i in lastIndex downTo 1) { ... } is already supported by optimizer
There was a problem hiding this comment.
Thanks @konsoletyper, that's exactly what I changed it to :)
| @@ -112,7 +133,7 @@ private fun <T> collectionsSort(list: MutableList<T>, comparator: Comparator<in | |||
|
|
|||
| array.asDynamic().sort(comparator.asDynamic().compare.bind(comparator)) | |||
|
|
|||
| for (i in 0..array.size - 1) { | |||
| for (i in array.indices) { | |||
There was a problem hiding this comment.
JS backend doesn't optimize such loops ATM.
There was a problem hiding this comment.
Thanks, changed it to 0..array.lastIndex
Contains low risk refactorings and formattings
…n JS Fixes: #KT-2460
|
I've rebased it to the latest 1.2 branch and run it on build server. The branch is 1.2-Beta2...rrr/1.2-Beta2/pr1300 If you don't mind I'll squash the first two commits. |
|
Sure :) |
|
I've merged it to the master, thanks for your effort. |
|
Thanks! |
Fixes: #KT-2460