protocol extension on MutableCollection Type does not work properly with slices

At this line:

var numbers3Slice: ArraySlice<Int> = numbers3[1..<4].shuffleInPlace()

you seem to expect that the shuffleInPlace()-method should return an ArraySlice<Int>, but it doesn't.

The shuffleInPlace()-method clearly returns Void (see the (lack of) return type in your method declaration).

Latest Xcode (7.1) will give an error message on that line. I don't know why you are not getting that error.

This compiles and works as expected:

import Darwin

extension MutableCollectionType where Index == Int {
    mutating func shuffleInPlace() {
        let c = count + startIndex
        if count < 2 { return }
      
        for i in startIndex..<c - 1 {
            let j = Int(arc4random_uniform(UInt32(c - i))) + i
            guard i != j else { continue }
            swap(&self[i], &self[j])
        }
    }
}

var numbers3 = [1, 2, 3, 4, 5, 6, 7]
numbers3.shuffleInPlace()
print(numbers3) // Prints [6, 3, 1, 7, 5, 2, 4]

var rangeA = numbers3[1..<5]
rangeA.shuffleInPlace()
print(rangeA) // Prints [3, 1, 5, 7]

print(numbers3) // Original array unchanged as expected (since Arrays are value types).

This is the kind of confusion that arises when methods that return something look the same as those that don't. I believe shuffleInPlace is inspired by sortInPlace, which teaches us to overcomplicate.

sortInPlace is a verb. A verb should return nothing. ✅

sort is a verb. It should return nothing, but does. ✖

We have a better way to describe what sort does: sorted. Functions that return something should have noun or adjective names. When it's a noun, the dot operator is read as an apostrophe. When an adjective, the dot is a comma.

numbers' count

numbers, sorted

For numbers.sort(), the pronoun "yourself" is implied, for the object of the predicate:

numbers, sort

Of course, this is not quite enough, often due to deverbal nouns that aren't gerunds, e.g. grin, wind, cough. That is why I use the giftbox and bow emoji after my optional and nonoptional nouns, respectively.

You are absolutly right. I need to return Self

extension MutableCollectionType where Index == Int {
    mutating func shuffleInPlace() -> Self {  // ADD SELF
        /
        let c = count + startIndex
        if count < 2 { return self}                   // ADD SELF

        for i in startIndex..<c - 1 {
            let j = Int(arc4random_uniform(UInt32(c - i))) + i
            guard i != j else { continue }
            swap(&self[i], &self[j])
        }
         return self   // ADD SELF
    }
}

Now, it's solved all my problems. Now i can use slices as I expert. No error at all.

var sString = ["1","4","3","8","9","10"]
sString[1..<4].shuffleInPlace()                      // OutPut: ["3", "4", "8"]
var sStringSlice = sString[1..<4].shuffleInPlace()   // OutPut: ["4", "3", "8"]
sStringSlice.shuffleInPlace()                        // OutPut: ["8", "4", "3"]

Returning self is considered bad practice. Languages in which you would return self would also return a value from the assignment operator.

Marking your own bad-practice-answer as correct can perhaps also be considered bad practice.

Yes, good points.

Sorry. I am too fast. But how my code will be look with your recommendation? I have Xcode 7.1, but i don't have error in line with ArraySlice.

I need

var rangeA = numbers3[1..<5].shuffleInPlace()

I didn't mean to make a big deal out of it, and I'm not sure about Apple's naming conventions, but I think most would agree that it's not good practice to have a method called shuffleInPlace which does what it says AND returns anything but Void. And I don't think renaming such a method to shuffledInPlace would be very nice either (it's just strange to have a method that both mutates in place and returns a copy of self ...)

Writing this in a more conventional way would probably result in something like the following implementation by Nate Cook (link to original code on github omitted to avoid having to wait for moderation):

// (c) 2015 Nate Cook, licensed under the MIT license
//
// Fisher-Yates shuffle as protocol extensions

extension CollectionType {
    /// Return a copy of `self` with its elements shuffled
    func shuffle() -> [Generator.Element] {
        var list = Array(self)
        list.shuffleInPlace()
        return list
    }
}

extension MutableCollectionType where Index == Int {
    /// Shuffle the elements of `self` in-place.
    mutating func shuffleInPlace() {
        // empty and single-element collections don't shuffle
        if count < 2 { return }

        for i in 0..<count - 1 {
            let j = Int(arc4random_uniform(UInt32(count - i))) + i
            guard i != j else { continue }
            swap(&self[i], &self[j])
        }
    }
}

[1, 2, 3].shuffle()
// [2, 3, 1]

let fiveStrings = stride(from: 0, through: 100, by: 5).map(String.init).shuffle()
// ["20", "45", "70", "30", ...]

var numbers = [1, 2, 3, 4]
numbers.shuffleInPlace()
// [3, 2, 1, 4]

And it could probably be nice to prepend shuffle with a

    @warn_unused_result(mutable_variant="shuffleInPlace")

as can be seen in the std lib for eg sort.

And according to Jessy's reasoning about function/method naming (which I agree with, except maybe for the emoji-stuff), the method shuffle should be renamed shuffled.

But as already mentioned, Nate Cook's code is in accordance with (what seems to be) Apple's naming conventions, although I really can't understand the reasoning behind them, ie chosing eg sort over sorted, shuffle over shuffled, etc. If anyone can explain them or have pointers to related documention I would appreciate it.

Thank you very much for such detail answer. I realy take this code from stackoverhere. But Nate Cook's code does not work for ArraySlice, which confirms MutableCollectionType, because inXcode 7.1 "

ArraySlice

Before

var a = Array(0..<10)
var s = a[5..<10]
s.indices // 0..<5

After

var a = Array(0..<10)
var s = a[5..<10]
s.indices // 5..<10

....the Swift standard library provides “slicing,” which composes well with algorithms. This enables you to write

a[3..<7].sortInPlace()

I made little changes Nate Cook's code:

. . . . . . .
let c = count + startIndex
 if count < 2 { return } 
  for i in startIndex..<c - 1 {
. . . . . . . .

and research shuffleInPlace (it doesn't matter now what is it name) for ArraySlice

numbers3[1..<4].shuffleInPlace()

It return () - no error.

If I use the same for Array

[1, 2, 3, 4, 5, 6, 7].shuffleInPlace()

I have error : cannot use mutating member on immutable value of type '[Int]'.

I don't understand, why there is a difference in behavior shuffleInPlace for Array and ArraySlice? When you implement protocol extension, it doesn't matter, which collection uses it's method. Probably, I am missing something. It seems to me, that something wrong.

You are correct in that Nate Cook's code above has to be modified for the new ArraySlice startIndex, endIndex behaviour. I corrected the code and added some examples with comments, see below.

Regarding the things you say you don't understand: I think you need to learn more in general about things like immutable, mutable, var, let, value type, reference type, literals, etc. Here are two examples of why I think so:

1. You seem to be surprised by the fact that shuffleInPlace returns Void / (). Which shouldn't be surprising given the name and the return type of shuffleInPlace.

numbers3[1..<4].shuffleInPlace()

What happens here is that numbers3[1..<4] creates an ArraySlice that is then mutated by shuffleInPlace and then thrown away into empty space. I'm not sure what you wanted to accomplish here, maybe you meant to do something like this:

let shuffledSlice = numbers3[1..<4].shuffled()

2. You're trying to use the mutating method shuffleInPlace on an array literal, but of course that won't work.

[1, 2, 3, 4, 5, 6, 7].shuffleInPlace()

Array literals are immutable and you are trying to call a mutating method on it, and the compiler is only doing its job telling you that you can't do that.

You can read the Swift iBook and watch related WWDC videos to learn more about these things and more.

Here's the working code example:

import Foundation

extension CollectionType {
    /// Return a copy of `self` with its elements shuffled
    @warn_unused_result(mutable_variant="shuffleInPlace")
    func shuffled() -> [Generator.Element] {
        var list = Array(self)
        list.shuffleInPlace()
        return list
    }
}
extension MutableCollectionType where Index == Int {
    /// Shuffle the elements of `self` in-place.
    mutating func shuffleInPlace() {
        // empty and single-element collections don't shuffle
        if count < 2 { return }
        for i in 0 ..< count - 1 {
            let j = Int(arc4random_uniform(UInt32(count - i))) + i
            guard i != j else { continue }
            swap(&self[startIndex + i], &self[startIndex + j]) // <-- Modified this line in order to support new behaviour of ArraySlice.
        }
    }
}
var numbers = [0, 1, 2, 3, 4, 5, 6] // We declare this as var as we will mutate it.
numbers.shuffleInPlace()            // The array is shuffled in place, method returns Void == ().
print(numbers)                      // Prints the array: [3, 5, 6, 1, 4, 2, 0]

let slice = numbers[3 ..< 7]         // Take a slice of the last four numbers, put it in a const/let since we do not want to mutate it.
let shuffledSlice = slice.shuffled() // Get a shuffled copy of slice, not mutating the original, returning the shuffled copy.
print(slice)         // Prints [1, 4, 2, 0] which is the last four elements of numbers.
print(shuffledSlice) // Prints [4, 1, 0, 2] which are the shuffled copy of that slice.

print(numbers) // Prints [3, 5, 6, 1, 4, 2, 0] again since numbers hasn't been mutated since it was shuffled using shuffledInPlace above.

Thank you very much. I realy need deeper knowledge.