0
\$\begingroup\$

There are 12 buttons, and each of them has 3 options for selecting pictures, depending on the data. It works but I think need a shorter and more intelligent decision.

import SpriteKit

class SelectEasyLevelViewController: UIViewController{

@IBOutlet weak var level_1: UIButton!
@IBOutlet weak var level_2: UIButton!
@IBOutlet weak var level_3: UIButton!
@IBOutlet weak var level_4: UIButton!
@IBOutlet weak var level_5: UIButton!
@IBOutlet weak var level_6: UIButton!
@IBOutlet weak var level_7: UIButton!
@IBOutlet weak var level_8: UIButton!
@IBOutlet weak var level_9: UIButton!
@IBOutlet weak var level_10: UIButton!
@IBOutlet weak var level_11: UIButton!
@IBOutlet weak var level_12: UIButton!

override func viewWillAppear(_ animated: Bool) {
    super.viewWillAppear(true)

    //This thing help me to open data
    if  UserDefaults.standard.array(forKey: "levelPassed") != nil {
        Model.sharedInstance.levelPassed = UserDefaults.standard.array(forKey: "levelPassed") as! [Int]
    }

let image = UIImage(named: "levelbutton01")
let image2 = UIImage(named: "levelbutton02")
let image3 = UIImage(named: "levelbutton03")

    switch Model.sharedInstance.levelPassed[0] {
    case 1:
        level1.setBackgroundImage(image, for: .normal)
    case 2:
        level1.setBackgroundImage(image2, for: .normal)
    case 3:
        level1.setBackgroundImage(image3, for: .normal)
    default:
        break

    }

    ..........................................
    Here codes for button 2, 4, 5, 6, 7, 8, 9, 10, 12
    .........................................

    switch Model.sharedInstance.levelPassed[11] {
    case 1:
        level_12.setBackgroundImage(image, for: .normal)
    case 2:
        level_12.setBackgroundImage(image2, for: .normal)
    case 3:
        level_12.setBackgroundImage(image3, for: .normal)
    default:
        break

    }
}

Or maybe another way.

\$\endgroup\$
4
  • \$\begingroup\$ at first: if you need to count your items then you have a possible code smell in programming ;) good that you ask for it \$\endgroup\$
    – muescha
    Commented Jul 23, 2018 at 23:54
  • \$\begingroup\$ at first use DRY -> create a method to do just the switch statement. and then use IBOutletCollection loop over the buttons and use the tag property of the button to set the level then use it in the loop \$\endgroup\$
    – muescha
    Commented Jul 23, 2018 at 23:57
  • \$\begingroup\$ 1) no switch statement is even needed, 2) This is all UIKit, not sprite kit \$\endgroup\$ Commented Jul 24, 2018 at 13:11
  • \$\begingroup\$ level1.setBackgroundImage(UIImage(named: "levelbutton\(String(format: "%02d", Model.sharedInstance.levelPassed[0]))"), for: .normal) \$\endgroup\$ Commented Jul 24, 2018 at 13:13

1 Answer 1

1
\$\begingroup\$

DRY - Don't Repeat Yourself:

1) change this block into an method:

switch Model.sharedInstance.levelPassed[0] {
    case 1:
        level1.setBackgroundImage(image, for: .normal)
    case 2:
        level1.setBackgroundImage(image2, for: .normal)
    case 3:
        level1.setBackgroundImage(image3, for: .normal)
    default:
        break
}

into this (assuming you can access thee image1 to image 3:

func applyImage(button: UIButton, level: Int){
    switch Model.sharedInstance.levelPassed[0] {
        case 1:
            level1.setBackgroundImage(image, for: .normal)
        case 2:
            level1.setBackgroundImage(image2, for: .normal)
        case 3:
            level1.setBackgroundImage(image3, for: .normal)
        default:
            break
    }
}

so you can call it this way:

applyImage(level_1,1)
applyImage(level_2,2)
applyImage(level_12,12)

2) avoid the switch:

arrays are zero based, so you decrease the level by 1

let images = [image1,image2,image3]

func applyImage(button: UIButton, level: Int, images: [UIImage]){
    button.setBackgroundImage(images[level-1], for: .normmal)
}
applyImage(level_1 , 1, images)
applyImage(level_2 , 2, images)
applyImage(level_12,12, images)

3) use IBOutletCollection and .tag property

because the order of an IBOutletCollection is not guaranteed you should use the tag property to ensure it shows the right information:

@IBOutlet var buttons: [UIButton]!

let images = [image1,image2,image3]

for button in buttons {
    button.setBackgroundImage(images[level-1], for: .normmal)    
}

PS: all code is untested pseudo code - but it should show you the idea how to improve your code

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.