5
\$\begingroup\$

Is this code missing any "Kotlinisms"?

I've just started learning Kotlin so I'm not yet familiar with the Kotlin way of doing things.

The program will guess a number you are thinking of.

package challenges.difficult

/**
 * Created by jbarnes on 04/06/2016.
 * Challenge: https://www.reddit.com/r/dailyprogrammer/comments/pii6j/difficult_challenge_1/
 */

fun main(args:Array<String>) {
    Guesser();
}

class Guesser(range:IntRange) {

    private var range = range;
    private var upperLimit:Int = range.last;
    private var lowerLimit:Int = range.first;
    private var turnCount:Int = 0;
    private var lastGuess:Int = 0;
    private var lastResponse:Response = Response.UNKNOWN;

    constructor():this(1..100)

    init {
        printIntro();
        while (lastResponse != Response.CORRECT) {
            lastResponse = presentNextGuess();
            turnCount++;
            if (lastResponse == Response.CORRECT) {
                println("I guessed your number in $turnCount turns.");
            }
            if (lastResponse == Response.UNKNOWN) {
                println("I'm sorry, I didn't understand that.");
            }
        }

    }

    fun calcNextGuess():Int {
        var roundingFactor = 0;
        if (lastResponse == Response.HIGHER) {
            lowerLimit = lastGuess;
            roundingFactor = 1; //compensates for rounding down
        } else if (lastResponse == Response.LOWER) {
            upperLimit = lastGuess;
        }
        return (lowerLimit + ((upperLimit - lowerLimit) / 2)) + roundingFactor;
    }

    fun presentNextGuess():Response {
        val nextGuess = calcNextGuess();
        println("Is your number $nextGuess?");
        val response = readLine();
        lastGuess = nextGuess;
        return parseResponse(response);
    }

    fun parseResponse(response:String?):Response {
        if (response == null) {
            return Response.UNKNOWN;
        } else if (response.equals("higher", true) || response.equals("h", true)) {
            return Response.HIGHER;
        } else if (response.equals("lower", true) || response.equals("l", true)) {
            return Response.LOWER;
        } else if (response.equals("yes", true) || response.equals("y", true)) {
            return Response.CORRECT;
        } else {
            return Response.UNKNOWN;
        }
    }


    fun printIntro() {
        println("Think of a number between 1 and 100. I am going to try and guess it.");
        println("If i guess correctly respond with 'yes' (or 'y')");
        println("Otherwise respond to with either 'higher' (or 'h') or 'lower' (or 'l')");
        println("Okay, here we go...");
    }

    enum class Response {
        HIGHER, LOWER, CORRECT, UNKNOWN
    }
}

https://github.com/JNBarnes/LearningKotlin/blob/master/src/challenges/difficult/GuessingGame.kt

\$\endgroup\$
6
\$\begingroup\$

Some suggestions:

  • Semi-colons are optional so, avoid them if possible.

  • Use type inference when possible. For example:

    val aInteger = 1 // Int is inferred by the compiler as the type of `aInteger`
    
    val aListOfInt = listOf(1, 2) // List<Int> is inferred by the compiler as the type of `aListOfInt`
    
  • Constructors, in particular primary constructor, are very powerful. For example: class Guesser(val range: IntRange = 1..100) is equivalent to:

    class Guesser(range: IntRange) {
      val range = range
    
      constructor(): this(1..100)
    }
    
  • val is for read-only properties. If you need to re-assign a property, use var instead.

  • The use of raw strings with the trimMargin() member function is preferable over multiple println invocations.

  • The when expression is similar to the more traditional switch. You can learn more about it here.

This is the whole thing:

class Guesser(val range: IntRange = 1..100) {
  private var upperLimit = range.last
  private var lowerLimit = range.first
  private var turnCount = 0
  private var lastGuess = 0
  private var lastResponse = Response.UNKNOWN

  init {
    printIntro()

    while (lastResponse != Response.CORRECT) {
      lastResponse = presentNextGuess()
      turnCount++

      when (lastResponse) {
        Response.CORRECT -> println("I guessed your number in $turnCount turns.")
        Response.UNKNOWN -> println("I'm sorry, I didn't understand that.")
      }
    }
  }

  fun calcNextGuess(): Int {
    var roundingFactor = 0

    when (lastResponse) {
      Response.HIGHER -> {
        lowerLimit = lastGuess
        roundingFactor = 1 //compensates for rounding down
      }
      Response.LOWER -> {
        upperLimit = lastGuess
      }
      else -> assert(false) // Assume error if this is reached
    }

    return (lowerLimit + ((upperLimit - lowerLimit) / 2)) + roundingFactor
  }

  fun presentNextGuess(): Response {
    val nextGuess = calcNextGuess()

    println("Is your number $nextGuess?")

    val response = readLine()

    lastGuess = nextGuess

    return parseResponse(response)
  }

  fun parseResponse(response: String?) = when (response?.toLowerCase()) {
    null -> Response.UNKNOWN
    "higher", "h" -> Response.HIGHER
    "lower", "l" -> Response.LOWER
    "yes", "y" -> Response.CORRECT
    else -> Response.UNKNOWN
  }

  fun printIntro() {
    println(
      """
        Think of a number between 1 and 100. I am going to try and guess it.
        If i guess correctly respond with 'yes' (or 'y')
        Otherwise respond to with either 'higher' (or 'h') or 'lower' (or 'l')
        Okay, here we go...
      """.trimIndent()
    )
  }

  enum class Response {
    HIGHER, LOWER, CORRECT, UNKNOWN
  }
}

fun main(args:Array<String>) {
  Guesser()
}

Note: I'm also learning the language and I could be wrong with some things so, the above code can be improved and is not free of mistakes and non-idiomatic constructs.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ You should explain why your version is better than the op's one. \$\endgroup\$ – Denis Jun 5 '16 at 10:05
  • 2
    \$\begingroup\$ @denis: I included several comments in the code with that purpose. \$\endgroup\$ – EPadronU Jun 5 '16 at 10:08
  • 2
    \$\begingroup\$ Hi. Welcome to Code Review! Given that we have a format that allows the intermixing of code and text, it is often better to put explanations in text. Only put explanations in code comments if you would naturally comment there in the program. For example, /* If you're going to reassign the members, they must be var` /` is probably *not something that you'd normally put in real code. By contrast, Assume error if this is reached is a perfectly natural comment. \$\endgroup\$ – mdfst13 Jun 5 '16 at 10:48
  • 1
    \$\begingroup\$ Kotlin when on enum is recommended to be exhaustive but does not have to be. \$\endgroup\$ – mfulton26 Jun 6 '16 at 16:26
  • 1
    \$\begingroup\$ parseResponse can be a single when expression as well. e.g. fun parseResponse(response: String?) = when (response?.toLowerCase()) { null -> Response.UNKNOWN "higher", "h" -> Response.HIGHER "lower", "l" -> Response.LOWER "yes", "y" -> Response.CORRECT else -> Response.UNKNOWN } \$\endgroup\$ – mfulton26 Jun 6 '16 at 16:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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