Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

iOS

Posted by Mitch Little
Mitch Little
11,870 Points

How to eliminate the duplication of this code.

Hi, I have created a practice version of the app we create with Pasan.

In this view controller I have created 4 If Else statements. The app works perfectly however it seems unnecessary to have this many statements and goes ages the DRY convention.

I have tried experimenting to find the solution such as adding multiple execution statements, and using a switch statement instead however I cannot seem to find the solution.

import UIKit

class ViewController: UIViewController {

    @IBOutlet weak var headsOrTailsLabel: UILabel!

    @IBOutlet weak var headsOrTailsButton: UIButton!

    let headsOrTailsModel = HeadsOrTailsModel() // Remember to assign the data model to a new constant or variable. Call any methods on this new constant or variable as methods cannot be called on a Struct itself.
    let colourForHeads = BackgroundColour().headsColour // Assigning a stored property of background colour to a constant.
    let colourForTails = BackgroundColour().tailsColour

    override func viewDidLoad() {
        super.viewDidLoad()

        headsOrTailsLabel.text = headsOrTailsModel.headsOrTails()

        if headsOrTailsLabel.text == "Heads" { // If Else statement to alter background colour of the main view
            view.backgroundColor = colourForHeads
        } else {
            view.backgroundColor = colourForTails
        }

        if headsOrTailsLabel.text == "Heads" { // If Else statement to alter tint of headsOrTailsButton
            headsOrTailsButton.tintColor = colourForHeads
        } else {
            headsOrTailsButton.tintColor = colourForTails
        }
    }

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

    @IBAction func showHeadsOrTails() { // Ensure each sent evemt is only attacthed to one method.
        headsOrTailsLabel.text = headsOrTailsModel.headsOrTails()

        if headsOrTailsLabel.text == "Heads" { //If Else statement to alter background colour of the main view
            view.backgroundColor = colourForHeads
        } else {
            view.backgroundColor = colourForTails
        }

        if headsOrTailsLabel.text == "Heads" { // If Else statement to alter tint of headsOrTailsButton
            headsOrTailsButton.tintColor = colourForHeads
        } else {
            headsOrTailsButton.tintColor = colourForTails
        }

    }
}

Any other general advice would of course, be greatly appreciated.

Thank you,

Mitch

2 Answers

Steve Hunter
Steve Hunter
57,712 Points

Hi Mitch,

The test seems to be the same, i.e. if something equals "Heads", so there's no need for a switch statement which is a good way of dealing with multiple options.

You can have more than one line in each if/else part:

        if headsOrTailsLabel.text == "Heads" { // If Else statement to alter background colour of the main view
            view.backgroundColor = colourForHeads
            headsOrTailsButton.tintColor = colourForHeads
        } else {
            view.backgroundColor = colourForTails
            headsOrTailsButton.tintColor = colourForTails
        }

        //if headsOrTailsLabel.text == "Heads" { // If Else statement to alter tint of headsOrTailsButton
        //    headsOrTailsButton.tintColor = colourForHeads
        //} else {
        //    headsOrTailsButton.tintColor = colourForTails
        //}

But you then seem to repeat all of this inside another function. So, create a new func just for that. I'll call it changeColor. Something like:

func changeColor() {
        if headsOrTailsLabel.text == "Heads" { // If Else statement to alter background colour of the main view
            view.backgroundColor = colourForHeads
            headsOrTailsButton.tintColor = colourForHeads
        } else {
            view.backgroundColor = colourForTails
            headsOrTailsButton.tintColor = colourForTails
        }
}

Then something like this in viewDidLoad:

    override func viewDidLoad() {
        super.viewDidLoad()

        headsOrTailsLabel.text = headsOrTailsModel.headsOrTails()

        changeColor();
    }

// and at the second point:

    @IBAction func showHeadsOrTails() { // Ensure each sent evemt is only attacthed to one method.
        headsOrTailsLabel.text = headsOrTailsModel.headsOrTails()

        changeColor()
    }

Something along those lines should reduce the amount of duplication. Usually, if you see duplication; a new method is one way of being DRY.

I hope that helps; I didn't run your code in anything to test my suggestion - it is just that, a suggestion, which I hope helps you get to the solution you want.

Steve.

Steve Hunter
Steve Hunter
57,712 Points

You could put the other repeated line in the method at the start too:

func changeColor() {
    headsOrTailsLabel.text = headsOrTailsModel.headsOrTails() // Added this too
    if headsOrTailsLabel.text == "Heads" { 
        view.backgroundColor = colourForHeads
        headsOrTailsButton.tintColor = colourForHeads
    } else {
        view.backgroundColor = colourForTails
        headsOrTailsButton.tintColor = colourForTails
    }
}

Although that would probably make sense to change the method name

Mitch Little
Mitch Little
11,870 Points
Mitch Little
Mitch Little
11,870 Points

Awesome!

Nice one Steve, thank you for your help.

Mitch

Steve Hunter
Steve Hunter
57,712 Points

No problem - happy to help! :+1: :smile: