Prime number checker returns the same result each time

Question

I'm a beginner programmer learning Swift and made a basic prime number checker. No matter what it will only give one result, instead of changing based on wether or not the number is prime. Any help would be appreciated.

@IBAction func primeCheck(sender: AnyObject) {

var numberInt  = number.text.toInt()
var isPrime = true

    if number != nil {        
        if numberInt == 1 {      
            isPrime = false
        }

        if numberInt != 1 {         
            for var i = 2; i < numberInt; i++ {        
                if numberInt! % i == 0 {
                    isPrime = false
                } else {
                    isPrime = true
                }
            }
        }

    }

    if isPrime == true {
        result.text = "\(numberInt!) is a prime number!"
    } else {
        result.text = "\(numberInt!) is not a prime number!"
    }

}

Answer 1

I have another possible solution. At first I divide by two because it cannot be a prime number. Then you loop until the number is prime or the number divided by two is less than the divider.

@IBAction func primeCheck(sender: AnyObject) {

    var numberInt  = number.text.toInt()
    var isPrime = true
    var divider = 3

    if number < 2 || (number != 2 && number % 2 == 0) {
        isPrime = false
    }

    // you only have to check to half of the number
    while(isPrime == true && divider < number / 2){
        isPrime = number % divider != 0
        divider += 2
    }

    if isPrime == true {
        result.text = "\(numberInt!) is a prime number!"
    } else {
        result.text = "\(numberInt!) is not a prime number!"
    }
}

Answer 2

The error in your logic comes in this section:

if numberInt! % i == 0 {
    isPrime = false
} else {
    isPrime = true
}

At the top of your function, you initialize isPrime to be true, so in your loop you only need to look for cases that prove the number is not prime. You don't ever need to set isPrime = true again, so just drop the else condition:

if numberInt! % i == 0 {
    isPrime = false
}

Answer 3

You actually have two functions here. One to check if a number is prime and the other to display the result. Separating these makes everything much easier to manage.

// function to check primality and return a bool
// note that this can only accept a non optional Int so there is
// no need to check whether it is valid etc...
func checkNumberIsPrime(number: Int) -> Bool {
    // get rid of trivial examples to improve the speed later
    if number == 2 || number == 3 {
        return true
    }

    if number <= 1 || number%2 == 0 {
        return false
    }

    // square root and round up to the nearest int
    let squareRoot: Int = Int(ceil(sqrtf(Float(number))))

    // no need to check anything above sqrt of number
    // any factor above the square root will have a cofactor
    // below the square root.
    // don't need to check even numbers because we already checked for 2
    // half the numbers checked = twice as fast :-D
    for i in stride(from: 3, to: squareRoot, by: 2) {
        if number % i == 0 {
            return false
        }
    }

    return true
}

// function on the button. Run the check and display results.
@IBAction func primeCheck(sender: AnyObject) {
    let numberInt? = numberTextField.text.toInt() // don't call a text field "number", it's just confusing.

    if let actualNumber = numberInt {
        if checkNumberIsPrime(actualNumber) {
            resultLabel.text = "\(actualNumber) is a prime number!" // don't call a label "result" call it "resultLabel". Don't confuse things.
        } else {
            resultLabel.text = "\(actualNumber) is not a prime number!"
        }
    } else {
        resultLabel.text = "'\(numberTextField.text)' is not a number!"
    }
}

It makes it all much easy to read and maintain.

Answer 4

Well i don't know about swift, but maybe this is wrecking your code:

if numberInt! <<

To do a faster algorithm you could just search for divisors from 2 to sqrt(numberInt). (Theorem)

Answer 5

You have to break out of the loop after you find that the number is divisble by another number. Also for prime check you only have to check the divisibility till the square root of the number.

You can also use optional binding to extract numberInt and check for nil. That's the swift way.

@IBAction func primeCheck(sender: AnyObject) {

    var isPrime = true
    if let numberInt  = number.text.toInt() {

        if numberInt == 1 {
            isPrime = false /
        }
        else // Add else because you dont have to execute code below if number is 1
        {
            if numberInt != 1 {
                for var i = 2; i * i <= numberInt; i++ { // Only check till squareroot
                    if numberInt % i == 0 {
                        isPrime = false
                        break // Break out of loop if number is divisible.

                    } // Don't need else condition because isPrime is initialised as true.
                }
            }
        }

       if isPrime {
          result.text = "\(numberInt) is a prime number!"
       } else {
          result.text = "\(numberInt) is not a prime number!"
       }
    }

}

Reason for square root check : Why do we check up to the square root of a prime number to determine if it is prime?

You can refine the code further by refactoring the prime check into a separate function.

func isPrime(number:Int) -> Bool
{
    if number == 1 {
        return false
    }
    else
    {
        if number != 1 {
            for var i = 2; i * i <= numberInt; i++ {
                if numberInt % i == 0 {
                    return false
                }
            }
        }
    }
    return true
}

@IBAction func primeCheck(sender: AnyObject) {

    if let numberInt  = number.text.toInt() {

        if isPrime(numberInt) {
            result.text = "\(numberInt) is a prime number!"
        } else {
            result.text = "\(numberInt) is not a prime number!"
        }
    }

}