1
\$\begingroup\$

Please give me any feedback regarding good coding standards and readability.

fun main(arg: Array<String>) {

    //do while user guess != Mersenne Prime.
    do {
        //initializing variable
        var number: Int = 3
        try {
            number = readLine()!!.toInt()

            if (isMersennePrime(number)) 
            println("${number} is a Mersenne Prime. Congratulations.")

            else 
            println("${number} is not a Mersenne Prime.")
        }
        catch (e: NumberFormatException) {
            println(" is not a valid number. Exiting.")
        }
    } while (!isMersennePrime(number))
}

    // is Number both prime and 2^n - 1? 
    fun isMersennePrime(Number: Int): Boolean =    
    (twonminus1(Number.toDouble()) && isPrime(Number))

    // is x Prime?
    fun isPrime(x: Int): Boolean = (2..x-1).all{x % it != 0 && x != 2}

    // is x = 2^n - 1?
    fun twonminus1(x: Double): Boolean = Math.log(x + 1.0)/Math.log(2.0) % 1.0 == 0.0
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

Others already made some good points. Calling isMersennePrime twice is really bad because that can be very computationally intensive.

I'm not sure why you are not allowing 2 to be a prime number. Where I'm from 2 is prime.

When scanning for primes, you can stop at sqrt(x) instead of x - 1. If y * z = x, one of y or z is smaller than sqrt(x).

@Roland gave you a trick on checking if an integer is a power of two, but I just wanted to add a reference to wikipedia with some explanation.

Also var number: Int = 3 is quite odd. It's better to use immutable val. Instead:

while (true) {
    print("Enter a number: ")
    val text = readLine()
    val number = try {
        text!!.toInt()
    } catch (e: NumberFormatException) {
        println("$text is not a valid number. Exiting.")
        break
    }
    if (isMersennePrime(number)) {
        println("${number} is a Mersenne Prime. Congratulations.  Exiting.")
        break
    } else {
        println("${number} is not a Mersenne Prime.  Try again.")
    }
}
\$\endgroup\$
2
\$\begingroup\$

Do not use double arithmetics when integer arithmetics can solve the problem as well. The function isPowerOfTwoMinus1 can be written simply as ((x + 1) & x) == 0.

Don't call isMersennePrime twice, just break from the loop.

\$\endgroup\$
0
1
\$\begingroup\$
  1. (2..x-1).all{x % it != 0 && x != 2} can be replaced with (3..x-1).all{x % it != 0}
  2. Your variables should follow camelCase. You have a Number variable in isMersennePrime
  3. Your try-catch could cover only the number = readLine... line
\$\endgroup\$
1
  • \$\begingroup\$ Your first point is not quite true. You don't want the number to be divisible by 2, so you should not start at 3. I'm not sure why there is this x != 2 in the original post however. \$\endgroup\$
    – toto2
    Aug 8, 2017 at 19:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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