1

The intention of the code is to produce a random exercise and number of reps (selected from an array) every time the button is pushed.

However, the button will do this only the first time it is pressed, and not again thereafter.

Question : how can I ensure the button will generate a random number of reps and exercise (chosen from my arrays), each time it is pushed?

import UIKit

class ViewController: UIViewController

{

    var clickCount = 0

    let exercises = ["Push Ups", "Squats", "Burpees", "Sit Ups"]
    let reps = ["5", "6", "7", "8", "9", "10"]

    lazy var randomIndex1 = Int(arc4random() % UInt32(exercises.count))
    lazy var randomIndex2 = Int(arc4random() % UInt32(reps.count))

    @IBOutlet weak var countLabel: UILabel!
    @IBOutlet weak var excerciseType: UILabel!
    @IBOutlet weak var repVolume: UILabel!

    @IBAction func buttonPress(_ sender: UIButton) {

        clickCount+=1

        countLabel.text="You've Tapped \(clickCount) times"

        excerciseType.text="\(exercises[randomIndex1])"

        repVolume.text="\(reps[randomIndex2])"

    }

}
picciano
  • 22,341
  • 9
  • 69
  • 82
Shaun
  • 35
  • 7
  • Move the random indexes inside your button action function. The way you have it now, they are initialized lazily the first time you use them and the number wont change. – pkorosec Jan 09 '18 at 21:07
  • Recommendation. Instead of using `arc4random()`, which could lead to [modulo bias](https://stackoverflow.com/questions/10984974/why-do-people-say-there-is-modulo-bias-when-using-a-random-number-generator), use [arc4random_uniform](https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man3/arc4random_uniform.3.html) instead. – Miket25 Jan 09 '18 at 21:18

3 Answers3

2

Your lazy initialized variables don't change their values after the lazy initialization.

Use computed variables instead which return new values on each access:

var randomIndex1 : Int {
    return Int(arc4random_uniform(UInt32(exercises.count))
}

var randomIndex2 : Int {
    return Int(arc4random_uniform(UInt32(reps.count))
}

The code uses the more appropriate API arc4random_uniform

vadian
  • 274,689
  • 30
  • 353
  • 361
  • While this could work it's not needed. Just make the variables local to the function not class instances. If the values are not needed outside of the function don't make them available and if they were then they will be different every time they are accessed which is a bit pointless. – Upholder Of Truth Jan 09 '18 at 21:21
  • Not *could*, will . I just wanted to be as close as possible to the question. – vadian Jan 09 '18 at 21:25
  • True and it works fine and there are certainly times when this is appropriate. For example it allows the method of calculation of randomIndex1 and randomIndex2 to be changed without anything affecting it that uses it. So it does have advantages but if the function using it is isolated then I think it's best to keep as much of the related code together for readability if nothing else. – Upholder Of Truth Jan 09 '18 at 21:27
2

Generate a new value by putting the variables inside of the buttonPress method

So, I would remove these two lines

   lazy var randomIndex1 = Int(arc4random() % UInt32(exercises.count))
   lazy var randomIndex2 = Int(arc4random() % UInt32(reps.count))

and set those values inside the buttonPress

 @IBAction func buttonPress(_ sender: UIButton) {

   clickCount+=1
   let randomIndex1 = Int(arc4random() % UInt32(exercises.count))
   let randomIndex2 = Int(arc4random() % UInt32(reps.count))

   countLabel.text="You've Tapped \(clickCount) times"

   excerciseType.text="\(exercises[randomIndex1])"

   repVolume.text="\(reps[randomIndex2])"

 }

Now it should generate a new random value every time the button gets pressed. As Leo says in the comments, since we are regenerating them each time, they should be let and not var

Note

I originally mentioned something about lazy vars since they seem to be not needed here. So, if you read the comments and wonder what people are talking about; here is what I originally said, which doesn't really help the OP with the question and wasn't clear; so I'm moving it down here to this note:

A few things:

  • a lazy var is actually a constant (or at least it gets treated like one in a lot of places)
Community
  • 1
  • 1
Walter
  • 5,867
  • 2
  • 30
  • 43
  • Can you cite that "a lazy var is actually a constant"? – Miket25 Jan 09 '18 at 21:13
  • A lazy var is not a constant. All a lazy var is is a var where it's initial value is not calculated until it is actually needed. A lot of people treat them as constant but that is not enforced in any way and not how they are designed. Also inside the function randomIndex1 and randomIndex 2 can be defined as let instead of var because they are never mutated. Other than that the test is correct. – Upholder Of Truth Jan 09 '18 at 21:16
  • hmm.. Here is the discussion in the most recent apple documentation. https://developer.apple.com/library/content/documentation/Swift/Conceptual/Swift_Programming_Language/Properties.html Let me also think of how to reword that. The use of lazy by the OP isn't really the issue but I just was remarking on it. Maybe I'll take that part out fo my answer. – Walter Jan 09 '18 at 21:17
  • Yes, what @UpholderOfTruth says better than what I said. We use 'lazy' when we don't want to create the thing until it's used and in many cases it is for a constant since it is not allowed to have a 'lazy let' – Walter Jan 09 '18 at 21:18
  • Now you should declare `randomIndex1` and `randomIndex2` as constants using let instead of var – Leo Dabus Jan 09 '18 at 21:24
  • 1
    btw better to use arc4random_uniform. `Int(arc4random_uniform(UInt32(exercises.count)))` – Leo Dabus Jan 09 '18 at 21:26
  • Yes, @LeoDabus totally correct. Since we're creating them each time they aren't vars anymore. – Walter Jan 09 '18 at 21:26
  • And no need to use string interpolation. Btw you don't even need a var and/or constant for that `excerciseType.text = exercises[Int(arc4random_uniform(UInt32(exercises.count)))]` – Leo Dabus Jan 09 '18 at 21:28
1

To make your randomizer reusable you should move it into a new function. For the sake of this answer I will call it randomize(). You could implement like this:

import UIKit

class ViewController: UIViewController {

  var clickCount = 0

  let exercises = ["Push Ups", "Squats", "Burpees", "Sit Ups"]
  let reps = ["5", "6", "7", "8", "9", "10"]

  lazy var randomIndex1 = Int(arc4random() % UInt32(exercises.count))
  lazy var randomIndex2 = Int(arc4random() % UInt32(reps.count))

  @IBOutlet weak var countLabel: UILabel!
  @IBOutlet weak var excerciseType: UILabel!
  @IBOutlet weak var repVolume: UILabel!

  @IBAction func buttonPress(_ sender: UIButton) {
    clickCount+=1
    countLabel.text="You've Tapped \(clickCount) times"
    self.randomize()
  }

  func randomize() {
    self.randomIndex1 = Int(arc4random() % UInt32(exercises.count))
    self.randomIndex2 = Int(arc4random() % UInt32(reps.count))
    self.excerciseType.text="\(exercises[randomIndex1])"
    self.repVolume.text="\(reps[randomIndex2])"
  }

}
creeperspeak
  • 5,403
  • 1
  • 17
  • 38
  • It doesn't need to be a separate function if all that is needed is for it to be used when the button is clicked but it won't hurt. However the real issue is that randomIndex1 and randomIndex2 don't need to be class variables but could be local to the function (unless they are needed somewhere else too) – Upholder Of Truth Jan 09 '18 at 21:20
  • @UpholderOfTruth You are right. I suppose this is just one option. The computed properties answer is ultimately the best - I upvoted that one myself. – creeperspeak Jan 09 '18 at 21:21
  • I have just commented on an the computed one and I would actually go down the route of variables local to the function (which is why I upvoted that answer). – Upholder Of Truth Jan 09 '18 at 21:22
  • So the same as this answer, but just keeping all code within the button selector? I guess I just figured he may want to call the code from elsewhere, but I suppose that was never stated in the question. – creeperspeak Jan 09 '18 at 21:24