Make number of recent values a parameter #3604
Conversation
Allows easier experimentation for streetcomplete#3593. Also, I suspect that the right values are different between the building type and levels quest, so we'll want to do this later, anyway.
| */ | ||
| fun <T : Any> Sequence<T?>.mostCommonWithin(target: Int, historyCount: Int): Sequence<T> { | ||
| fun <T : Any> Sequence<T?>.mostCommonWithin(target: Int, historyCount: Int, first: Int): Sequence<T> { |
There was a problem hiding this comment.
I wonder if this signature would be more understandable as
fun <T : Any> Sequence<T?>.mostCommonWithin(first: Int, top: Int, within: Int): Sequence<T> {So instead of mostCommonWithin(6, 30, 1) you'd do mostCommonWithin(1, 5, 30).
I guess it's unintuitive in the edge case of "what if the first entry is null and we return 6 top values instead".
|
This only needs a cursory review, I considered just merging it immediately, but I had the idea I commented above, and @FloEdelmann you seem to like bikeshedding with me about code style :). If you don't have an opinion or don't care, feel free to just merge. |
|
I am not convinced that it should be configurable in a normal release. |
|
Maybe I used the wrong word here. It's not end-user configurable. I just added a parameter to the |
smichel17
changed the title
|
On second thought, while I remain not-totally-happy with the interface here, I also don't want to spend any more time on this. It's definitely "good enough". |
Allows easier experimentation for #3593. Also, I suspect that the right values are different between the building type and levels quest, so we'll want to do this later, anyway.