0

I was told my code contains a lot of force unwrapping. I thought it's okay to do that if I am sure that the value operated won't be nil:

private var x: Int?
private var y: Int?

@IBAction func startButtonPressed(_ sender: UIButton) {
    
    guard let numberOfRooms = selectedRooms.text, !numberOfRooms.isEmpty else {
        return selectedRooms.placeholder = "type it, dude"
    } 
    let rooms = Int(numberOfRooms)
                   
    x = Int(ceil(sqrt(Double(rooms!))))
    y = x //grab some values from user input
    maze = MazeGenerator(x!, y!) //generate a maze 
    hp = 2 * (x! * y!) //get hp value depending on user input
    
    currentX = getRandomX(x!) //get random value in 0...x
    currentY = getRandomY(y!)
    currentCell = maze?.maze[currentX!][currentY!] //game starts in a random part of the maze
                            
    refreshButtons() //refresh UI
    maze!.display() //print maze scheme in debug console
    }

Does it look fine? If not, what should be done?

pawello2222
  • 46,897
  • 22
  • 145
  • 209
  • Never undestimate users imagination :) Your `rooms` can be nil if users type a letter instead of a number. The answer by PGDev resolved it with `let rooms = Double(numberOfRooms)` in the first `guard`. You "may" be sure that your data and calculatoin are not nil, but if a user action is required… brace yourself :) – Alrik Aug 31 '20 at 09:26
  • Sorry for the two comment: in addition, you have not the problem of negative number as X is calculated with a sqrt, but you can have the problem of a too big number for an Int to be stored (there can also be issue if your maze is too big, even if it can be calculated maybe became useless for the game). So I will put also a `let x = Int(ceil(sqrt(rooms))) < avalueyouthinkisok` – Alrik Aug 31 '20 at 09:35

1 Answers1

3

It is recommended to avoid over using the force-unwrapping the optional variables.

Even if you're sure, the code might generate nil values in some corner cases that you might have not expected earlier. This will then result in unnecessarily crashing your app and giving a bad user experience.

Try using Optional Binding if-let / guard-let statement to unwrap your optionals safely.

@IBAction func startButtonPressed(_ sender: UIButton) {
    
    guard let numberOfRooms = selectedRooms.text, !numberOfRooms.isEmpty, let rooms = Double(numberOfRooms) else {
        return selectedRooms.placeholder = "type it, dude"
    }
    
    self.x = Int(ceil(sqrt(rooms)))
    self.y = x //grab some values from user input
    
    guard let x = self.x, let y = self.y else {
        return
    }
    
    maze = MazeGenerator(x, y)
    hp = 2 * (x * y)
    
    //for rest of the code I need to know the declaration of currentX, currentY, maze variables
}
PGDev
  • 23,751
  • 6
  • 34
  • 88
  • for my better understanding. Since `y = x` i need to guard both variable or is enought to guard only `x`? If x pass, also will do y, right? – Alrik Aug 31 '20 at 09:39
  • `self.y = x //grab some values from user input` Since you wrote this comment, it is not clear what you want `y` to be. – PGDev Aug 31 '20 at 10:00
  • I'm not the creator of the tread :) Assuming the code is as written (so X is a value inserted by users, and Y is = to X), I can guard only one of the variables or I should to both anyway? (for instance, bad memory allocation can actualy make X ok and Y no?) – Alrik Aug 31 '20 at 10:19
  • 1
    @Alrik Use if-let for every optional. You don't know where y might be getting set as nil. Its more safer this way – PGDev Aug 31 '20 at 10:23