Check if a number is prime

Question

I'm looking for feedback on the following code which checks if a number is prime using Swift 2.

 import UIKit
 var number:Int = 1123
 var isPrime:Bool = true
 switch number {
 case 1:
 isPrime = false
 case 2:
 isPrime = true
 case 3:
 isPrime = true
 default:
 primeCheck:for i in 2...Int(sqrt(Double(number))) {
 if number % i == 0 {
 isPrime = false
 break primeCheck
 }
 }
}
if isPrime {
 print("The number \(number) is prime!")
} else {
 print("The number \(number) is composite!")
}

• user86624: Should we assume that number is never smaller than 1? Because Int can be.
• Average: Ah, good point, no that is a mistake on my part, I should be checking for numbers less than 1.
• user86624: I also wonder if you need a case for 3, and even 2, since you already have initialized isPrime to true. You'll need to change the upper bound for the for loop a bit then: 2...Int(sqrt(Double(number)))+1, but it loses two case labels. Matter of taste perhaps.
• nhgrif: This article includes a really good isPrime implementation, as well as a discussion on how to unit test these sorts of functions.

Answer 1

I won't comment on the algorithm you're using to find out if the number is prime or not. Though the way you're doing it is quite inefficient. Some people in the comments have already pointed out some possible improvements and I'd suggest looking into the sieve of Eratosthenes to get better performance. You can find a Swift implementation of it here.

First off, there is a lot of unnecessary whitespace, you should get rid of that and only use a blank line too separate logical parts of your code.

Secondly, let's look at the following part:

var number:Int = 1123
var isPrime:Bool = true

There are three issues with it. number is mutable, but you never actually change it, it is customary to have exactly one space after a colon in Swift (instead of none) and while we're at it, in Swift you don't typically explicitly state the the of a variable unless it significantly improves readability (which is almost never) or type inference gets the wrong type. (which also doesn't happen often.) So you can rewrite those two lines as:

let number = 1123
var isPrime = true

As for the meat of this answer, it would be preferable to extract the actual prime checking into a function:

func isPrime(number: UInt32) -> Bool {
 switch number {
 case 0, 1: // you can put multiple cases on one line
 return false
 case 2, 3:
 return true
 default:
 for i in 2...Int(sqrt(Double(number))) {
 if number % i == 0 {
 return false
 }
 }
 return true
 }
 }
}

This allows you to write:

let number = 1123
if isPrime(number) {
 print("The number \(number) is prime!")
} else {
 print("The number \(number) is composite!")
}

or even:

print("The number \(number) is \(isPrime(number) ? "prime" : "composite")!")

This allows you to get rid of the mutable isPrime, which is something you should strive for in Swift. And allows you to nicely ecapsule the actrual logice and separate it from the rest.