0

Relatively new to Swift development. Trying to modify a couple functions someone else wrote. The goal is to leave the original pathway on the first time calling this after user login (that part hasn't been setup yet) and on subsequent calls, use another pathway.

So before writing the logic that will direct to first or non-first pass through, I am testing the non-first pass through logic.

Here's allOfferCards():

func allOfferCards() -> [OfferCard]{

    guard dataSource != nil else {
        return []
    }

    let numberOfCards = self.dataSource!.kolodaNumberOfCards(self)

    var offerCards = [OfferCard]()

    for i in 0..<numberOfCards {
        let offerCard = viewForCard(at: i)

        if let offerCard = offerCard {
            offerCards.append(offerCard as! OfferCard)
        }

    }

    return offerCards
}

And here is where I am trying to make the changes. The original logic reverses the return from allOfferCards(). I want to use a custom function called "shuffle" that will randomize the array.

func displayOfferCards() -> Void {
    // What was here originally
    //let offerCards = allOfferCards().reversed()
    var offerCards = allOfferCards().shuffle()

    for (index, offerCard) in offerCards.enumerated() {
        let delay = Double(index) * 0.2
        offerCard.display(delay: delay)
    }
}

Here's the shuffle function

extension Array
{
    /** Randomizes the order of an array's elements. */
    mutating func shuffle()
    {
        for _ in 0..<10
        {
            sort { (_,_) in arc4random() < arc4random() }
        }
    }
}

However when I try to run this I get the error in the title - can't use a mutating member on an immutable value. However I'm not sure why allOfferCards() is generating an immutable value - var offerCards is defined using a var keyword, not a let keyword - this should mean it is mutable correct?

What am I doing wrong here?

Steven Matthews
  • 9,705
  • 45
  • 126
  • 232
  • 1
    That's a very, very poor shuffle algorithm. `sort` doesn't compare every element to every other element, it would be terribly inefficient to do so. If it checks and sees that `a < b` and `c < d`, then it can check `b < c` to infer `a < b < c < d` without checking `a < c`, `a < d`, and so on... Look into using a proper Fisher-Yates shuffle: https://stackoverflow.com/a/24029847/3141234 – Alexander Jan 16 '18 at 18:52
  • @Alexander - Oh thanks, I will do that. – Steven Matthews Jan 16 '18 at 18:55
  • Also, the first function can be reduced into 2 simple lines: https://gist.github.com/amomchilov/df144a2bee19b627e2163254a42a8a59 – Alexander Jan 16 '18 at 18:57
  • I didn't write it originally - we're aware that there is a lot of logic that can be rewritten to be more efficient (and some already has). I'll take a look at this and see if it fits what we're planning on doing here. – Steven Matthews Jan 16 '18 at 18:59
  • I know the feeling of having to work on a codebase like that, where you don't want to disturb the working state too much. Luckily, that's why we have VCS, to keep a nice safety net against unintentional breaking changes. But small incremental changes like this really build up and lead to a substantial improvement in the long run. I essentially gave it it you for free, so may as well embrace it :p – Alexander Jan 16 '18 at 19:01
  • Yes, I did bookmark it! Don't worry I am definitely going to try it out and see! Please leave it up for at least a week or two if you would so I can test it and try it out :) – Steven Matthews Jan 16 '18 at 19:04
  • When using these methods, I am getting: Value of tuple type '()' has no member 'enumerated' – Steven Matthews Jan 16 '18 at 19:27
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/163277/discussion-between-alexander-and-andrew-alexander). – Alexander Jan 16 '18 at 19:46

2 Answers2

1

The issue is that before assigning the return value of a function, the value itself is immutable, so calling a mutating function on it cannot be directly done. You should assign the return value first to a mutable variable, then call the mutating function on it.

func displayOfferCards() -> Void {
    var offerCards = allOfferCards()
    offerCards.shuffle()

    for (index, offerCard) in offerCards.enumerated() {
        let delay = Double(index) * 0.2
        offerCard.display(delay: delay)
    }
}
Dávid Pásztor
  • 51,403
  • 9
  • 85
  • 116
  • This looks like it is working. I'm just going to test it and I'll check your answer box. – Steven Matthews Jan 16 '18 at 18:56
  • I'm getting Value of tuple type '()' has no member 'enumerated' when I do this. – Steven Matthews Jan 16 '18 at 19:38
  • I tested the code in a Playground (commenting out the unknown function such as `viewForCard` of course) and it compiles just fine. Are you sure you are using the exact same function? The type of `offerCards` is `[OfferCard]`, so using above code you couldn't be getting that error. – Dávid Pásztor Jan 16 '18 at 19:50
  • I'm absolutely using the functions I listed above. Could it be a Swift 3 thing? – Steven Matthews Jan 16 '18 at 22:05
  • 1
    Are you sure you are using the exact same question? Just because in your [new question](https://stackoverflow.com/questions/48290807/variable-offercardsshuffled-inferred-to-have-type-which-may-be-unexpecte), you do `var offerCardsShuffled = offerCards.shuffle()` which is wrong and is absolutely not what I've shown in my answer. – Dávid Pásztor Jan 16 '18 at 23:46
0

Supplementing @Dávid Pásztor answer.

Instead of using a mutating functions (shuffle()), you can use a varient that returns a shuffled copy (shuffled()), similar to sort() vs sorted(). Since there's no variable being mutated, it can all be done in-line:

func displayOfferCards() {
    for (index, offerCard) in allOfferCards().shuffled().enumerated() {
        offerCard.display(delay: Double(index) * 0.2)
    }
}

See this answer for an implementation of shuffle(), shuffled().

Alexander
  • 59,041
  • 12
  • 98
  • 151