<div dir="ltr">Thank you for working on this!</div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Dec 17, 2017 at 11:07 PM, Saagar Jha via swift-users <span dir="ltr"><<a href="mailto:swift-users@swift.org" target="_blank">swift-users@swift.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><br><div>
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">Saagar Jha</div>
</div>
<div><span class=""><br><blockquote type="cite"><div>On Dec 17, 2017, at 22:24, Nevin Brackett-Rozinsky via swift-users <<a href="mailto:swift-users@swift.org" target="_blank">swift-users@swift.org</a>> wrote:</div><br class="m_-7882981136661945613Apple-interchange-newline"><div><div dir="ltr">…well that was more complicated than I anticipated.<div><br></div><div>The “random” function was (Int) -> Int, so when I removed the “IndexDistance == Int” constraint on “shuffle” I either had to add several “numericCast” calls or make “random” generic.</div><div><br></div><div>So I made “random” generic. And also fixed the modulo bias issue in the Glibc version that Saagar mentioned. Or at least I hope I did. I haven’t tested that part (nor any of the non-Swift-4 / non-macOS parts). Also, I am not sure why “random” had been a closure value stored in a “let” constant, but I changed it to a function.</div></div></div></blockquote><div><br></div></span><div>Great! I'll close my pull request, then.</div><br><blockquote type="cite"><div><span class=""><div dir="ltr"><div><br></div><div>While I was at it, I removed the random-access constraint I had placed on “shuffle”. It will now shuffle any MutableCollection, with complexity O(n) for a RandomAccessCollection and O(n²) otherwise. (The constraint was different in different Swift versions, so the entire extension had to be duplicated. Without the constraint the implementation is much sleeker.)</div><div><br></div><div>Nevin</div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Dec 17, 2017 at 9:26 PM, Nevin Brackett-Rozinsky <span dir="ltr"><<a href="mailto:nevin.brackettrozinsky@gmail.com" target="_blank">nevin.brackettrozinsky@gmail.<wbr>com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div class="m_-7882981136661945613h5"><div class="gmail_extra"><div class="gmail_quote">On Sun, Dec 17, 2017 at 7:37 PM, Dave Abrahams <span dir="ltr"><<a href="mailto:dabrahams@apple.com" target="_blank">dabrahams@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><br><blockquote type="cite"><span class="m_-7882981136661945613m_-2884960459926388617gmail-"><div>On Dec 16, 2017, at 4:34 PM, Nevin Brackett-Rozinsky via swift-users <<a href="mailto:swift-users@swift.org" target="_blank">swift-users@swift.org</a>> wrote:</div><br class="m_-7882981136661945613m_-2884960459926388617gmail-m_2667904589485617490Apple-interchange-newline"></span><span class="m_-7882981136661945613m_-2884960459926388617gmail-"><div><div style="font-family:HelveticaNeue;font-size:14px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><font face="monospace, monospace">public extension MutableCollection where Self: RandomAccessCollection, IndexDistance == Int {</font></div></div></span></blockquote></div><br><div>IndexDistance == Int is an over-constraint, FWIW. Adding it is generally a mistake. Not a serious one, but it does limit utility somewhat.</div><div><br></div><div>HTH,</div><div>Dave</div></div></blockquote></div><br></div></div></div><div class="gmail_extra">You are correct, however there is an accepted-and-implemented proposal (<a href="https://github.com/apple/swift-evolution/blob/master/proposals/0191-eliminate-indexdistance.md" target="_blank">SE–0191</a>) to eliminate IndexDistance and replace it with Int, so the constraint will always be satisfied starting in Swift 4.1.</div><div class="gmail_extra"><br></div><div class="gmail_extra">I wrote a version of shuffle() without that constraint, but it is less elegant: the line “for n in 0 ..< count - 1” no longer compiles, and writing “0 as IndexDistance” doesn’t fix it because the resulting Range is not a Sequence, since IndexDistance.Stride does not conform to SignedInteger (at least in Swift 4.0 according to Xcode 9.2).</div><div class="gmail_extra"><br></div><div class="gmail_extra">A while loop works of course, though a direct conversion requires writing “var n = 0 as IndexDistance” before the loop. Luckily, with a bit of cleverness we can eliminate all mention of IndexDistance, and this time we really and truly don’t need the “guard !isEmpty” line:</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra"><font face="monospace, monospace">extension MutableCollection where Self: RandomAccessCollection {</font></div><div class="gmail_extra"><font face="monospace, monospace"> mutating func shuffle() {</font></div><div class="gmail_extra"><font face="monospace, monospace"> </font><span style="font-family:monospace,monospace">var n = count</span></div><div class="gmail_extra"><font face="monospace, monospace"> while n > 1 {</font></div><div class="gmail_extra"><font face="monospace, monospace"> let i = index(startIndex, offsetBy: count - n)</font></div><div class="gmail_extra"><font face="monospace, monospace"> let j = index(i, offsetBy: random(n))</font></div><div class="gmail_extra"><span style="font-family:monospace,monospace"> n -= 1</span><font face="monospace, monospace"><br></font></div><div class="gmail_extra"><font face="monospace, monospace"> swapAt(i, j)</font></div><div class="gmail_extra"><font face="monospace, monospace"> }</font></div><div class="gmail_extra"><font face="monospace, monospace"> }</font></div><div class="gmail_extra"><font face="monospace, monospace">}</font></div></div><div class="gmail_extra"><br></div><div class="gmail_extra">Essentially, the constraint is there to make the code nicer on pre-4.1 versions of Swift, though I’m happy to remove it if you think that’s better. If nothing else, removing the constraint means people reading the example code in the future won’t be misled into thinking they need to use it themselves, so perhaps it should go just on that account.</div><div class="gmail_extra"><br></div><div class="gmail_extra">Okay, you’ve convinced me, I’ll update the PR. :-)</div><span class="m_-7882981136661945613HOEnZb"><font color="#888888"><div class="gmail_extra"><br></div><div class="gmail_extra">Nevin</div></font></span></div>
</blockquote></div><br></div></div></span><span class="">
______________________________<wbr>_________________<br>swift-users mailing list<br><a href="mailto:swift-users@swift.org" target="_blank">swift-users@swift.org</a><br><a href="https://lists.swift.org/mailman/listinfo/swift-users" target="_blank">https://lists.swift.org/<wbr>mailman/listinfo/swift-users</a><br></span></div></blockquote></div><br></div><br>______________________________<wbr>_________________<br>
swift-users mailing list<br>
<a href="mailto:swift-users@swift.org">swift-users@swift.org</a><br>
<a href="https://lists.swift.org/mailman/listinfo/swift-users" rel="noreferrer" target="_blank">https://lists.swift.org/<wbr>mailman/listinfo/swift-users</a><br>
<br></blockquote></div><br></div>