SOLVED: Challenge 4: Improvements on my solution?

@GutoP

Nov '21

Hi Guys, hope you are all doing great! I wanted to share my solution for the 4th challenge with you,and check if there is any improvement to be made here:

enum myErrors: Error{
    case outOfBounds, noMatch
}

func root(of number: Int) throws -> Int {
    if number < 1 || number > 10000 { throw myErrors.outOfBounds}

    var found: Int = 0

    for i in 1...100{
        if ((i*i) == number) {
            found = i
        }
    }

    if found == 0 {throw myErrors.noMatch}

    return found
}

do{
    let result = try root(of: 9)
    print("your result is \(result)")
} catch myErrors.noMatch {
    print("This number does not have an integer square root")
} catch myErrors.outOfBounds{
    print ("this is out of bounds")
}

Thanks!

@Obelix

Nov '21

Welcome to Challenge 4!

Well done!

Your code works, so you're on the right path.

Can your code be improved? Sure however...

There are minor tweaks you can make here and there. You will start to see these in Paul's tutorials, other YouTube videos, Slack channels, and github projects. Then you'll start seeing where your code can be improved.

But what are the improvements? As far as speed, iOS devices have tremendously powerful chips. They process millions of tasks each second listening for Siri commands, counting your steps, processing background networks packets, syncing your device with iCloud, calculating your location, furiously unpacking stored data and turning it into music, detecting finger swipes, etc, etc. Refactoring an IF statement in Challenge 4 won't make your code any more efficient, or faster.

Your goal is to make your code readable and understandable. One day you'll reach back into an older program and stare blankly wondering "What was I thinking??" Future GutoP will thank you if you make the effort to code concisely and clearly. You won't master this immediately. But work on it.

Here are a few examples. You wrote:

for i in 1...100 {  // using i for a counter was common practice with languages like Basic.
        if ((i*i) == number) {
            [...snip.....]

You are trying to find the root of a number. "i" is not very descriptive of what this number represents in your solution. What would be a better name for this variable?

You coded:

for i in 1...100{
        if ((i*i) == number) {
            found = i
        }
    }

    if found == 0 {throw myErrors.noMatch}
    return found

Why take the extra step to assign the i variable to found ? Plus, if your number is 16, and you found the solution is 4, your code still processes all the possibilities of 5 through 100. Why? Instead, consider simplifying your loop.

for root in 1...100 {
        if ((root * root) == number) {
            return root  // this is the number you've been looking for!
        }
    }

// if you make it this far, you've exhausted all possibilites.
// Your only choice is to throw an error.
throw myError.noMatch

In one of Paul's videos he stated the best way to avoid bugs in your code, is to not write a lot of code. (or something similar). By eliminating the found variable and its IF statements, you have fewer places in your code where logic errors may hide.

Keep coding! We want to see your progress in the upcoming challenges!

@GutoP

Nov '21

thanks @Obelix for the tips and encouragement on my journey! As a beginner i tend to fell a little doubtful of myself... but your words have helped a lot!

Have a great one.

Sponsor Hacking with Swift and reach the world's largest Swift community!

Archived topic

This topic has been closed due to inactivity, so you can't reply. Please create a new topic if you need to.

All interactions here are governed by our code of conduct.