6
\$\begingroup\$

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<UIButton>?

    @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 resetGame(){
        score = 0
        turnCount = 0
        correctRun = 0
        playAgainButton.hidden = true
        queryParameters = ["None"]
        queryHoles = ""
        displayRandomMember()
    }

    func displayRandomMember(){
        for button in choiceButtons! {
            button.setBackgroundImage(yellowButton, forState: .Normal)
            button.userInteractionEnabled = true
        }

        let querySQL = "SELECT name, picture_name from member_data where picture_name is not ? and name not in (\(queryHoles)) ORDER BY RANDOM() LIMIT 1";
        rightAnswer = memberDatabase!.executeQuery(querySQL, withArgumentsInArray: queryParameters)
        rightAnswer!.next()
        correctName = rightAnswer!.stringForColumn("name")!

        let correctPicture = rightAnswer!.stringForColumn("picture_name")
        let wrongAnswerSQLQuery = "SELECT name from member_data where picture_name is not ? and name is not ? ORDER BY RANDOM() LIMIT 3" //gets three wrong answers
        let wrongAnswersResultSet:FMResultSet = memberDatabase!.executeQuery(wrongAnswerSQLQuery, withArgumentsInArray: ["None", correctName!])
        var wrongAnswersArray:[String] = []
        while wrongAnswersResultSet.next() == true {
            wrongAnswersArray.append(wrongAnswersResultSet.stringForColumn("name"))
        }
        var wrongButtons = [firstChoice, secondChoice, thirdChoice, fourthChoice]
        correctButton = wrongButtons.removeAtIndex(Int(arc4random_uniform(4)))
        rotateButton(correctButton!, newname: correctName!)
        for i in 0..<wrongButtons.count{
            rotateButton(wrongButtons[i], newname:wrongAnswersArray[i])
        }

        memberPic.image = UIImage(named: correctPicture)
        queryParameters.append(correctName!) //keep track of names already seen this game
        if (count(queryHoles) > 0) {
            queryHoles += (",?") //if already has one '?'
        } else {
            queryHoles += ("?")
        }
        return
    }

    func checkAnswer(sender:AnyObject){
        let selectedAnswer = sender.currentTitle!
        for button in choiceButtons! {
            button.userInteractionEnabled = false
        }
        correctButton!.setBackgroundImage(greenButton, forState: .Normal)
        if selectedAnswer! == correctName! {
            correctRun += 1
            score += (100 * correctRun)
            scoreLabel.text = "\(score)"
        } else {
            sender.setBackgroundImage(redButton, forState: .Normal)
            correctRun = 0
        }
        let delayTime = dispatch_time(DISPATCH_TIME_NOW, Int64(NSEC_PER_SEC * 1))
        dispatch_after(delayTime, dispatch_get_main_queue()){

            self.turnCount += 1
            if self.turnCount < 10 {
                self.displayRandomMember()
            } else {
                self.checkHighScore()
                self.playAgainButton.hidden = false
            }
        }

    }

    func checkHighScore(){
        let defaults = NSUserDefaults.standardUserDefaults()
        var oldHighScore = defaults.integerForKey("highScore")
        if (score > oldHighScore) {
            defaults.setInteger(score, forKey: "highScore")
        }
    }

    func rotateButton(button:UIButton, newname:String) {

        UIView.transitionWithView(
            button,
            duration: 0.5,
            options: UIViewAnimationOptions.TransitionFlipFromLeft | .AllowAnimatedContent,
            animations: {button.setTitle(newname, forState:.Normal)},
            completion: nil )
        }

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


}

Questions:

  • I feel like I am supposed to close the database at some point, but I'm not sure where.
  • I hate how I query to make sure I'm not repeating names within a round (10 questions). I append "?" to queryholes and the name to queryParameters; it works, and it's the answer I got from this question but it feels so hackish.

enter image description here

\$\endgroup\$
6
\$\begingroup\$
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.

\$\endgroup\$
  • \$\begingroup\$ So, unsure about the nature of the Member class. The only info is the person's name and picture. Would there be a performance advantage in this method, or are you just saying it's cleaner? \$\endgroup\$ – thumbtackthief Jul 14 '15 at 17:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.