Multiple Choice Guessing Game

I've built an app for members of an organization I'm in--the app shows the picture of a member with four multiple choice buttons, and the user then tries to guess the member's name. Pretty simple, but this is one of my first major apps.

```
import UIKit
import Foundation

class ViewController: UIViewController {

@IBOutlet weak var memberPic: UIImageView!
@IBOutlet weak var scoreLabel: UILabel!
@IBOutlet weak var firstChoice: UIButton!
@IBOutlet weak var secondChoice: UIButton!
@IBOutlet weak var thirdChoice: UIButton!
@IBOutlet weak var fourthChoice: UIButton!
@IBOutlet var choiceButtons: Array?

@IBOutlet weak var playAgainButton: UIButton!

@IBAction func guessChosen(sender: AnyObject) {
checkAnswer(sender)
}
let redButton = UIImage(named: "red_button") as UIImage?
let greenButton = UIImage(named: "green_button") as UIImage?
let yellowButton = UIImage(named: "yellow_button") as UIImage?
var rightAnswer:FMResultSet?
var databasePath:String?
var correctName:String?
var correctButton:UIButton?
var memberDatabase:FMDatabase?

var score = 0
var correctRun = 0
var turnCount = 0
var queryParameters = ["None"] //"None" to exclude empty pics, names to be added
var queryHoles = "" //append "?" for each already-seen name

@IBAction func playAgainPressed(sender: AnyObject) {
resetGame()
}

override func viewDidLoad() {
super.viewDidLoad()
self.view.backgroundColor = UIColor(patternImage: UIImage(named: "background.jpeg")!)
let path = NSBundle.mainBundle().pathForResource("members", ofType:"sqlite3")
playAgainButton.hidden = true
memberDatabase = FMDatabase(path: path)
if memberDatabase!.open(){
println("database is ready")
} else {
println("error finding database")
}
scoreLabel.text = "\(score)"
resetGame()
//close database?

}

func res

let redButton = UIImage(named: "red_button") as UIImage?
let greenButton = UIImage(named: "green_button") as UIImage?
let yellowButton = UIImage(named: "yellow_button") as UIImage?

The as UIImage? is 100% unnecessary here. UIImage(named:) already is defined as returning this type. Adding as UIImage? just clutters your code.

override func didReceiveMemoryWarning() {
    super.didReceiveMemoryWarning()
    // Dispose of any resources that can be recreated.
}

If all you're doing in a method is calling super (and the method isn't marked as required), then keeping it around does nothing but add clutter to your code.

I realize that this particular method is part of the UIViewController template that Xcode generates for you, but that's no excuse to not remove the clutter (Xcode won't create it again in this file).

You have a lot of code in viewDidLoad. Move it out into logical, individual methods that make sense. Keep your life-cycle methods clean, clear, and simple. I don't want to have to read through a lot of code to figure out what's happening during a view controller's life cycle. I want to be able to compartmentalize chunks of code, and that starts with you breaking this down into smaller blocks that make logical sense.

A good viewDidLoad() method might look like this:

override func viewDidLoad() {
    super.viewDidLoad()
    setupUI()
    loadDatabase()
    resetGame()
}

You should be able to figure out what logic goes in each method.

Your displayRandomMember() method also has a lot of logic in it. And I'm not entirely convinced that all of it is strictly related to what's necessary to displaying a random member. I think some of it is just tangentially related, or rather, simply stuff you'd like to do around the same time as you're picking a random member.

Your code in general could use an OOP tune-up. You should have a Member class. Or something like this, which holds the necessary information about your members.

And then, rather than querying your database twice to get the "right" answer and then three "wrong" answers, why don't you just query it once to get four members, and then from those four, randomly pick one to be the correct answer?

So, I don't know what all information you need/want about your members, but assuming you have appropriately set up a Member class, we might want some flexible code like this:

func fetchRandomMembers(count: Int) -> [Member] {
    // return an array filled with 'count' members
    // members randomly selected from database
}

func setupGame() {
    let maxChoices = 4
    let choices = fetchRandomMembers(maxChoices)
    let correctChoice = choices[Int(arc4random_uniform(maxChoices))]

    // setup one button for each member in choices array
    // correctChoice is a Member instance
}

You should get the gist of the direction this is headed, right?

Depending on how big the database is (how many members) and how much information you have for each member, you might consider simply querying the database a single time to fill an array with all the members, and then using Swift to worry about randomly selecting your right & wrong guesses.

StackExchange Code Review Q#96397, answer score: 6