Refactor Shuffle in Get-Random to save a full iteration of the objects.#8969
Refactor Shuffle in Get-Random to save a full iteration of the objects.#8969iSazonov merged 7 commits intoPowerShell:masterfrom st0le:users/gakama/RefactorGetRandom
Conversation
src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs
Outdated
Show resolved
Hide resolved
| object tmp = _chosenListItems[i]; | ||
| _chosenListItems[i] = _chosenListItems[j]; | ||
| _chosenListItems[j] = tmp; | ||
| _chosenListItems[j] = _chosenListItems[i]; |
There was a problem hiding this comment.
Doesn't this cause the value in i to be duplicated? Shouldn't we swap like we did before?
There was a problem hiding this comment.
I understand now, the assignment effectively removes the output value from the list for the next iteration. The comment should be updated to make this more clear.
PaulHigin
left a comment
There was a problem hiding this comment.
Please update code comment to make assignment rather than swap change more clear.
| object tmp = _chosenListItems[i]; | ||
| _chosenListItems[i] = _chosenListItems[j]; | ||
| _chosenListItems[j] = tmp; | ||
| _chosenListItems[j] = _chosenListItems[i]; |
There was a problem hiding this comment.
I understand now, the assignment effectively removes the output value from the list for the next iteration. The comment should be updated to make this more clear.
|
How do I trigger the CI Build without a dummy commit? The build broke due to an unrelated change |
|
Just ask and a maintainer can do it for you. Otherwise it requires a dummy commit. In this scenario, asking was better. |
|
But looking at the history, it looks like GitHub is not remerging your PR. You might need to rebase your PR to reliably get a clean Windows CI. |
|
I can do that for you if you don't know how, but the failures I saw are intermittent. |
PR Summary
Minor refactoring of the Fisher Yates shuffle to save an additional iteration.
PR Context
Micro-optimization to the shuffle function.
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests