3
\$\begingroup\$

See this link for previous post on this subject Generating mathematical questions and answers

I have an Android quiz app that I am building to learn kotlin / android app development. Here is an object I have which acts as one of many question builders.

Since my last post I have:

  • broken down the main method into smaller methods as per advice given
  • renamed the variables/values so that they have better meaning
  • started using val instead of var where possible - Setting vals in when statements dont seem to work well for me, is this an example where var is best used like I do or is there a way to make vals work here too?
  • switched from using secure random which was not necessary for my use case
  • removed some duplication and even hard coded some values where previously I was using a variable which was being set and used in the same line of code

My object:

object test{
    fun newQ(context: Context,mainLevel:Int, subLevel:Int, isBonusRound:Boolean, gameType:String): SingleQuestion {
        val level = determineLevel(gameType, mainLevel) // Used to determine the level which can vary between mini games
        var numbers = initialiseNumbers() // Set an array of Int's to be used when creating a sum (question)
        val operations = determineOperations(level) // how many mathematical calculations will be needed to get to the correct answer?
        numbers = processNumbersStep2(isBonusRound,mainLevel,subLevel,numbers) //adjust numbers to suit level / bonus game
        val (question,answer) = createQuestion(context,isBonusRound,operations,numbers,level) //create the question and the answer
        val timeLimitPerQuestion = getTimeLimit(isBonusRound,operations,level) // how many seconds (as an Int) to give them to answer this question
        return SingleQuestion(question,answer,timeLimitPerQuestion) // return the question, the answer and the time limit
    }
}

// Function to provide a random number. Set the lowest number and highest number.
private fun rand(start: Int, end: Int): Int {
    require(start <= end) { "Illegal Argument" }
    val random = Random.Default
    return random.nextInt(start, end + 1)
}
// function to determine level due to differences in how mini games are structured (one had less levels so I map them here to the broader level structure)
private fun determineLevel(gameType:String,mainLevel:Int):Int{
    var updatedMainLevel = 0
    if (gameType=="FreeStyle"){
        when (mainLevel){
            1->{updatedMainLevel = rand(1,3)} // aka Easy
            2->{updatedMainLevel = rand(4,5)} // aka Normal
            3->{updatedMainLevel = rand(6,8)} // aka Hard
            4->{updatedMainLevel = rand(9,10)} // aka Impossible
            }
    } else {updatedMainLevel = mainLevel
    }; return updatedMainLevel
}
// create base numbers for use in sums, these may manipulated later depending on level etc
private fun initialiseNumbers() :Array<Int>{
    var num1 = 0;var num2 = 0;var num3 = 0;var num4 = 0
    when(rand(1,23)){
        1->{num1 = rand(1,5); num2 = rand(1,5)}
        2->{num1 = rand(2,6);num2 = rand(1,5)}
        3->{num1 = rand(3,7);num2 = rand(1,5)}
        4->{num1 = rand(4,8);num2 = rand(1,5)}
        5->{num1 = rand(5,9);num2 = rand(1,5)}
        6->{num1 = rand(6,10);num2 = rand(7,11)}
        7->{num1 = rand(1,5);num2 = rand(6,10)}
        8->{num1 = rand(2,6);num2 = rand(5,9)}
        9->{num1 = rand(3,7);num2 = rand(4,8)}
        10->{num1 = rand(4,8);num2 = rand(3,7)}
        11->{num1 = rand(5,9);num2 = rand(2,6)}
        12->{num1 = rand(6,10);num2 = rand(1,5)}
        13->{num1 = rand(1,5);num2 = rand(1,5)}
        14->{num1 = rand(2,6);num2 = rand(2,6)}
        15->{num1 = rand(3,7);num2 = rand(3,7)}
        16->{num1 = rand(4,8);num2 = rand(4,8)}
        17->{num1 = rand(5,9);num2 = rand(5,9)}
        18->{num1 = rand(6,10);num2 = rand(6,10)}
        19->{num1 = rand(1,5);num2 = rand(1,5)}
        20->{num1 = rand(1,5);num2 = rand(2,6)}
        21->{num1 = rand(1,5);num2 = rand(3,7)}
        22->{num1 = rand(1,5);num2 = rand(4,8)}
        23->{num1 = rand(1,5);num2 = rand(5,9)}
        else->{num1 = rand(1,5);num2 = rand(5,9)}
    }
    num3 = rand(2,6);num4 = rand(4,8)
    return arrayOf(num1,num2,num3,num4)
}
private fun determineOperations(level:Int):Int{
    var calculationsRequired = 0
    when(level){
        in 1..5-> calculationsRequired = 1
        in 6..10-> calculationsRequired = 2
        else -> calculationsRequired = 1
    }
    return calculationsRequired
}
private fun processNumbersStep2(isBonusRound: Boolean,mainLevel: Int,subLevel: Int,numbers:Array<Int>):Array<Int>{
    var num1=0;var num2=0;var num3=0;var num4=0
    if (!isBonusRound){
        when(mainLevel){
            1->{num1 = subLevelModifier(subLevel,numbers[0],0);num2 = subLevelModifier(subLevel,numbers[1],0)}
            2->{num1 = subLevelModifier(subLevel,numbers[0],1);num2 = subLevelModifier(subLevel,numbers[1],1)}
            3->{num1 = subLevelModifier(subLevel,numbers[0],2);num2 = subLevelModifier(subLevel,numbers[1],2)}
            4->{num1 = subLevelModifier(subLevel,numbers[0],3);num2 = subLevelModifier(subLevel,numbers[1],3)}
            5->{num1 = subLevelModifier(subLevel,numbers[0],4);num2 = subLevelModifier(subLevel,numbers[1],4)}
            6->{num1 = subLevelModifier(subLevel,numbers[0],0);num2 = subLevelModifier(subLevel,numbers[1],0);num3 = subLevelModifier(subLevel,numbers[0],0)}
            7->{num1 = subLevelModifier(subLevel,numbers[0],1);num2 = subLevelModifier(subLevel,numbers[1],1);num3 = subLevelModifier(subLevel,numbers[0],1)}
            8->{num1 = subLevelModifier(subLevel,numbers[0],2);num2 = subLevelModifier(subLevel,numbers[1],2);num3 = subLevelModifier(subLevel,numbers[0],2)}
            9->{num1 = subLevelModifier(subLevel,numbers[0],3);num2 = subLevelModifier(subLevel,numbers[1],3);num3 = subLevelModifier(subLevel,numbers[0],3)}
            10->{num1 = subLevelModifier(subLevel,numbers[0],4);num2 = subLevelModifier(subLevel,numbers[1],4);num3 = subLevelModifier(subLevel,numbers[0],4)}
        }
    } else {
        when(mainLevel){
            in 1..3->{num1 = subLevelModifier(subLevel,numbers[0],0);num2 = subLevelModifier(subLevel,numbers[1],0);num3 = subLevelModifier(subLevel,numbers[2],0);num4 = subLevelModifier(subLevel,numbers[3],0)}
            in 4..5->{num1 = subLevelModifier(subLevel,numbers[0],2);num2 = subLevelModifier(subLevel,numbers[1],2);num3 = subLevelModifier(subLevel,numbers[2],2);num4 = subLevelModifier(subLevel,numbers[3],2)}
            in 6..8->{num1 = subLevelModifier(subLevel,numbers[0],4);num2 = subLevelModifier(subLevel,numbers[1],4);num3 = subLevelModifier(subLevel,numbers[2],4);num4 = subLevelModifier(subLevel,numbers[3],4)}
            in 9..10->{num1 = subLevelModifier(subLevel,numbers[0],6);num2 = subLevelModifier(subLevel,numbers[1],6);num3 = subLevelModifier(subLevel,numbers[2],6);num4 = subLevelModifier(subLevel,numbers[3],6)}
        }
    }
    return arrayOf(num1,num2,num3,num4)
}
private fun createQuestion(context: Context,isBonusRound: Boolean,operations:Int,numbers:Array<Int>,level:Int):Pair<String,Int>{
    var question = "";var answer = 0
    if (!isBonusRound){
        when (operations){
            1-> {
                answer = numbers[0] * numbers[1]
                question = context.getString(R.string.mulQ_num1_x_num2,numbers[0],numbers[1])}
            2-> {
                answer = (numbers[0] * numbers[1]) * numbers[2]
                question = context.getString(R.string.mulQ_num1_x_num2_x_num3,numbers[0],numbers[1],numbers[2])}
            else->{
                answer = numbers[0] * numbers[1]
                question = context.getString(R.string.mulQ_num1_x_num2,numbers[0],numbers[1])}
        }
    } else {
        when (rand(1,4)){
            1->{
                answer = (numbers[0] * numbers[1]) + (numbers[2] * numbers[3])
                question = context.getString(R.string.mulQ_num1_x_num2_add_num3_x_num4,numbers[0],numbers[1],numbers[2],numbers[3])}
            2->{
                answer = numbers[0] + (numbers[1] * numbers[2]) + numbers[3]
                question =context.getString(R.string.mulQ_num1_add_num2_x_num3_add_num4,numbers[0],numbers[1],numbers[2],numbers[3])}
            3->{
                answer = ((numbers[0] * numbers[1]) + numbers[2]) - numbers[3]
                question =context.getString(R.string.mulQ_num1_x_num2_add_num3_minus_num4,numbers[0],numbers[1],numbers[2],numbers[3])}
            4->{
                answer = numbers[0] + (numbers[1] * numbers[2]) - numbers[3]
                question =context.getString(R.string.mulQ_num1_x_num2_add_num3_minus_num4,numbers[0],numbers[1],numbers[2],numbers[3])}
        }
    }
    return Pair(question,answer)
}
//This function is aimed at providing an optimisation capability.
// sb levels are 1-25 e.g sub level 1 increases each number by 1.01% up to 1.25% (1% increase to 25% increase)
// you can lower the division factor (e.g. /100 to /50) to create bigger gaps between sub levels
private fun subLevelModifier(subLevel:Int, num:Int, increment:Int):Int{
    var multiplier = ((subLevel /100 )+1).toDouble()
    var newNum = ((num + increment) * multiplier).roundToInt()
    return newNum
}
private fun getTimeLimit(isBonusRound: Boolean, operations: Int, level: Int): Int {
    var addTimeForBonusRound = 0;var addTimeForOperations = 0

    if (isBonusRound) {addTimeForBonusRound = +2}
    if (operations>1) {addTimeForOperations = +1}
    val addTimeForLevel = ((level*0.6)+2).roundToInt()
    return (addTimeForBonusRound+addTimeForOperations+addTimeForLevel)
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I have some suggestions that are in general more nitpicky than the answer to your previous question, since you've addressed most of that.

  1. Your comment says sublevel is in the range 0-25, but you divide it by 100 using integer math, so this line will evaluate to 1.0 every time.
var multiplier = ((subLevel /100 )+1).toDouble()

Also, multiplier should be a val and newNum eliminated since you can return the expression directly.

  1. Naming convention is for all classes (including object) to start with a capital letter, so your object should be named Test.

  2. determineOperations() is needlessly verbose with the use of the variable and when statement. It could just be:

private fun determineOperations(level: Int): Int{
    return if (level in 6..10) 2 else 1
}
  1. Your function rand() also sets up a variable just to use it one time. You can collapse such uses into simpler statements. Occasionally, it makes sense to create the variable so the following expression is easier to read, but this is of course not one of those cases. Also, Random.Default is a companion object of Random, so you can omit its name.
private fun rand(start: Int, end: Int): Int {
    require(start <= end) { "Illegal Argument" }
    return Random.nextInt(start, end + 1)
}
  1. processNumbersStep2() has a lot of code calling the same functions, and I think I see some patterns. I don't really understand your algorithm, but I think this would be equivalent to what you have now and carry less likelihood of typos and be easier to update if you need to later:
private fun processNumbersStep2(isBonusRound: Boolean,mainLevel: Int,subLevel: Int,numbers:Array<Int>):Array<Int>{
    if (!isBonusRound){
        val increment = mainLevel % 5
        val num1 = subLevelModifier(subLevel, numbers[0], increment)
        val num2 = subLevelModifier(subLevel, numbers[1], increment)
        val num3 = if (mainLevel <= 5) 0 else num1
        return arrayOf(num1, num2, num3, 0)
    } else {
        val increment = when(mainLevel){
            in 1..3 -> 0
            in 4..5 -> 2
            in 6..8 -> 4
            else -> 6
        }
        return numbers
            .map { subLevelModifier(subLevel, it, increment) }
            .toTypedArray()
    }
}

Maybe initialiseNumbers could be simplified in a similar way, but I don't see an easy way to break down num2.

  1. One of the design goals of Kotlin is to provide easy ways to make code more robust by avoiding mutability. List should be preferred to Array except when performance demands mutability. You are not currently even using the mutability provided by Array, so it would be very easy to swap in Lists.

  2. All your uses of context.getString could take advantage of the spread operator, for example:

question = context.getString(R.string.mulQ_num1_x_num2_add_num3_x_num4, *numbers)

You don't need to worry about some of the format strings not using the third and fourth values. They'll just be ignored. If you switch to List as I suggested in 6, you would use *numbers.toTypedArray().

  1. Your vars in getTimeLimit can be simplified into vals, such as:
private fun getTimeLimit(isBonusRound: Boolean, operations: Int, level: Int): Int {
    val addTimeForBonusRound = if (isBonusRound) 2 else 0
    val addTimeForOperations = if (operations > 1) 1 else 0
    val addTimeForLevel = ((level * 0.6) + 2).roundToInt()
    return addTimeForBonusRound + addTimeForOperations + addTimeForLevel
}
  1. Just a suggestion. Spaces around operators, inside brackets, and after commas help with readability. And multiple statements in a single line with semicolons is very bad for readability.
\$\endgroup\$
6
  • \$\begingroup\$ Thank you for this. I have read it through and can see where you are coming from, I'll review it properly when at my laptop and see what adjustments I can make \$\endgroup\$ Feb 3, 2021 at 11:35
  • \$\begingroup\$ Just one question, I'll test it of course myself to see what you are saying but where you mention integer maths does making it a double as part of the calculation do nothing? \$\endgroup\$ Feb 3, 2021 at 11:43
  • \$\begingroup\$ Based on your parentheses, you are only making it a double after division, so you are converting 1 to 1.0. \$\endgroup\$
    – Tenfour04
    Feb 3, 2021 at 13:23
  • \$\begingroup\$ ok so basically I need to do this? val multiplier = (subLevel.toDouble() /100 )+1 \$\endgroup\$ Feb 7, 2021 at 16:22
  • \$\begingroup\$ Yes. Personally, I like to be explicit, and would also use 100.0 and 1.0. The programming version of PEMBAS is more complicated and not consistent across all languages, so I think it's better practice not to rely on the rules of conversion order when they are rules that may vary from language to language. \$\endgroup\$
    – Tenfour04
    Feb 7, 2021 at 16:25

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.