# 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.

Is there a better way of writing this?

    package com.maxcell.sumitup

import java.security.SecureRandom
import kotlin.math.roundToInt

object MultiplicationFactory {

//What this code does by step
// step1 - I set 4 variables using a random number generator to different values. Sometimes the first variable is bigger, sometimes the second variable etc
// step 2 - based on the first constructor value (ML) the main level is passed to variable templevel
// This step provides sets a variable called vInc and with the constructor value (SL) is passed to function SLM()
// The function SLM takes three variables (SL, Num, vInc) and then adds vInc as a flat increase, the new value is then further increased
// by the value of SL but this is expressed as a percentage, the value is then returned as an updated integer
// step 3, the Num variables are then passed to this step to create a sum as a string (the question) and as an expression (the answer)
// step 4 the object function returns a dataclass containing 5 values for use in the activity:
// SingleQuestion(ML,vType,vQuestion,vAnswer,vTime) - Currently the values used from the return is the question and the answer, i then create
// three other answers, shuffle all of them and pass them to buttons: See screenshot for example of a question being used within the activity
// this code is used in different places in the app where a question is required under the specific minigame

var Num1 = 0 //Number for use in sums
var Num2 = 0 //Number for use in sums
var Num3 = 0 //Number for use in sums
var Num4 = 0 //Number for use in sums
var GrabQuestion = 0 //base level numbers
var Operations = 0 // how many calculations someone has to do to reach the answer

var vType = "" //e.g. multiplication, this is redundant soon to be redundant once all question types migrated
var vQuestion  = "" //The question as the user will see it
var vTime = 0 //Time to offer for the question
var vInc = 0 //sub level multiplier variable
var tempLevel = 0 //main level
var BSL = 0 // Bonus round selector

fun newQ(ML:Int, SL:Int,BR:Boolean,gameType:String):SingleQuestion {
vType = "Multiplication"

//map old game levels to new broader levelling logic - ignore this snippet in codereview
if (gameType=="FreeStyle"){
when (ML){
1->{tempLevel = rand(1,3)}
2->{tempLevel = rand(4,5)}
3->{tempLevel = rand(6,8)}
4->{tempLevel = rand(9,10)}
}} else {tempLevel = ML}
GrabQuestion = rand(1,23)

//Step 1
//to create a greater feel of randomness in the questions multiple possibilities created. These are base level figures
when(GrabQuestion){
1->{Num1 = rand(1,5);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
2->{Num1 = rand(2,6);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
3->{Num1 = rand(3,7);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
4->{Num1 = rand(4,8);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
5->{Num1 = rand(5,9);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
6->{Num1 = rand(6,10);Num2 = rand(7,11);Num3 = rand(2,6);Num4 = rand(4,8)}
7->{Num1 = rand(1,5);Num2 = rand(6,10);Num3 = rand(2,6);Num4 = rand(4,8)}
8->{Num1 = rand(2,6);Num2 = rand(5,9);Num3 = rand(2,6);Num4 = rand(4,8)}
9->{Num1 = rand(3,7);Num2 = rand(4,8);Num3 = rand(2,6);Num4 = rand(4,8)}
10->{Num1 = rand(4,8);Num2 = rand(3,7);Num3 = rand(2,6);Num4 = rand(4,8)}
11->{Num1 = rand(5,9);Num2 = rand(2,6);Num3 = rand(2,6);Num4 = rand(4,8)}
12->{Num1 = rand(6,10);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
13->{Num1 = rand(1,5);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
14->{Num1 = rand(2,6);Num2 = rand(2,6);Num3 = rand(2,6);Num4 = rand(4,8)}
15->{Num1 = rand(3,7);Num2 = rand(3,7);Num3 = rand(2,6);Num4 = rand(4,8)}
16->{Num1 = rand(4,8);Num2 = rand(4,8);Num3 = rand(2,6);Num4 = rand(4,8)}
17->{Num1 = rand(5,9);Num2 = rand(5,9);Num3 = rand(2,6);Num4 = rand(4,8)}
18->{Num1 = rand(6,10);Num2 = rand(6,10);Num3 = rand(2,6);Num4 = rand(4,8)}
19->{Num1 = rand(1,5);Num2 = rand(1,5);Num3 = rand(2,6);Num4 = rand(4,8)}
20->{Num1 = rand(1,5);Num2 = rand(2,6);Num3 = rand(2,6);Num4 = rand(4,8)}
21->{Num1 = rand(1,5);Num2 = rand(3,7);Num3 = rand(2,6);Num4 = rand(4,8)}
22->{Num1 = rand(1,5);Num2 = rand(4,8);Num3 = rand(2,6);Num4 = rand(4,8)}
23->{Num1 = rand(1,5);Num2 = rand(5,9);Num3 = rand(2,6);Num4 = rand(4,8)}
}
//when not a bonus round
//step2
if (BR==false){
//based on level increase base numbers (flat increase), based on sublevel increase numbers (%)
when (tempLevel){
1->{vInc = 0;Operations = 1;vTime = 10;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc)}
2->{vInc = 1;Operations = 1;vTime = 10;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc)}
3->{vInc = 2;Operations = 1;vTime = 10;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc)}
4->{vInc = 3;Operations = 1;vTime = 10;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc)}
5->{vInc = 4;Operations = 1;vTime = 10;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc)}
6->{vInc = 0;Operations = 2;vTime = 15;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc)}
7->{vInc = 1;Operations = 2;vTime = 15;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc)}
8->{vInc = 2;Operations = 2;vTime = 15;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc)}
9->{vInc = 3;Operations = 2;vTime = 15;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc)}
10->{vInc = 4;Operations = 2;vTime = 15;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc)}
}
//grab appropriate question format for number of calculations required
// step 3
when (Operations){
1->{vQuestion ="$Num1 \u00D7$Num2";vAnswer = Num1 * Num2}
2->{vQuestion ="($Num1 \u00D7$Num2) \u00D7 $Num3";vAnswer = (Num1 * Num2) *Num3} else->{vQuestion ="$Num1 \u00D7 $Num2";vAnswer = Num1 * Num2} } } else { //bonus games include different operators //step 2 when (tempLevel){ in 1..3->{vInc = 0;BSL = rand(1,4);vTime = 20;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc);Num4 = SLM(SL,Num4,vInc)} in 4..5->{vInc = 2;BSL = rand(1,4);vTime = 20;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc);Num4 = SLM(SL,Num4,vInc)} in 6..8->{vInc = 4;BSL = rand(1,4);vTime = 20;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc);Num4 = SLM(SL,Num4,vInc)} in 9..10->{vInc = 6;BSL = rand(1,4);vTime = 20;Num1 = SLM(SL,Num1,vInc);Num2 = SLM(SL,Num2,vInc);Num3 = SLM(SL,Num3,vInc);Num4 = SLM(SL,Num4,vInc)} } //grab appropriate question format for number of calculations required //step 3 when (BSL){ 1->{vQuestion ="($Num1 × $Num2) + ($Num3 x $Num4)";vAnswer = (Num1 * Num2) + (Num3 * Num4)} 2->{vQuestion ="$Num1 + ($Num2 ×$Num3) + $Num4";vAnswer = Num1 + (Num2 * Num3) + Num4} 3->{vQuestion ="(($Num1 × $Num2) +$Num3) - $Num4";vAnswer = ((Num1 * Num2) + Num3) - Num4} 4->{vQuestion ="$Num1 + ($Num2 ×$Num3) - \$Num4";vAnswer = Num1 + (Num2 * Num3) -Num4}
}
}
//return question to the minigame
// step 4
}
}
// 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 = SecureRandom()
random.setSeed(random.generateSeed(20))

return random.nextInt(end - start + 1) + start
}
//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 SLM(subLevel:Int, num:Int, increment:Int):Int{
var multiplier = ((subLevel / 100)+1).toDouble()
var newNum = ((num + increment) * multiplier).roundToInt()
return newNum
}


The screenshot shows a division question and my code is from a multiplication question builder but it's just to show how the data is used once returned

• Please provide some more details, not in code but in pure English, about what your code is doing. Can you provide some examples? Screenshots? Anything to make the question clearer. See codereview.meta.stackexchange.com/a/6429/31562 Commented Jan 11, 2021 at 13:59
• I'll add some more info shortly to show the steps the code goes through. Screenshots not possible as this is an object class that returns a question which is then dealt with in a separate activity Commented Jan 11, 2021 at 14:27
• @UnknownError Sorry for being unclear, my recommendation was about adding the "What this code does by step" part outside of the code block (unless you actually want to have it as comments inside the code, in which case parts of a review could potentially address these comments as well). Commented Jan 11, 2021 at 18:27
• Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. Commented Jan 19, 2021 at 15:46
• @UnknownError Yes, absolutely Commented Jan 19, 2021 at 19:29

BSL, ML, SL, BR, vInc, etc. might mean something to you, but it's not clear what it means by just looking at the names.

Example:

var BSL = 0 // Bonus round selector

This would be much clearer as

var bonusRoundSelector = 0

You have done the same thing for a lot of your variables. In some cases it's even contradictory.

var tempLevel = 0 //main level

Is it temporary or main? Those are two very different things.

Variables should be named so that they don't require a comment next to them about what their purpose is.

### Declaring variables

All your variables are declared outside of the function scope, that is unnecessary, they can be moved to inside the fun newQ instead.

Prefer using val instead of var whenever possible in Kotlin. You might get an error message from it but the solution to errors is to understand them and not workaround them just because you know how to workaround them.

There's multiple steps to be involved but here's some things that I notice:

• Num1 should be called num1 but you might want to try and come up with a different name for it as the four Num-variables does not seem very similar to me.
• If we would make Num1 a val we would be told that it cannot be reassigned. That is because you give it a default value of 0. This already is a problem, what if GrabQuestion is not in the range of 1..23 ? You might think that it always is because of you calling rand(1, 23), but what if there would one day be a bug in the rand function? Solutions: Use an else statement in when to throw an exception and don't initialize Num1 to anything, declare it as val num1: Int.
• Num1 is mutated again in step2, all these steps could be extracted as different methods and their data stored in one or several data class structures. For example, val numbers = initializeNumbers(grabQuestion) to initialize the num1, num2, num3, num4 values.

### Copied code

This is just an example, but BSL is always set to rand(1,4) so take it out of the duplication in the when and only do it once.

The same goes for vTime, Num1, and many other values.

Additionally, you have several magic numbers copy-pasted in your code, things like vTime = 20 could maybe be vTime = TIME_LONG where TIME_LONG is declared as const val TIME_LONG = 20

### SecureRandom

First of all, it's not necessary to set the seed on SecureRandom, it can do that by itself.

Secondly, I don't think it's necessary to use SecureRandom here at all. It's a Math Quiz, not a password generation service. Use Random.Default instead like this:

val random = Random.Default
return random.nextInt(start, end + 1)


### Overall

The current code is difficult to read and difficult to maintain, using better variable naming and less code duplication and structuring the code into multiple separated methods for each step, this will improve drastically.

I started refactoring the code but realized that it was a bit of work and that you wouldn't learn much by copy-pasting my code, but I promise you that your main method can be reduced to something like this:

val vType = "Multiplication"
val level = determineLevel(gameType, ML)
val grabQuestion: Int = rand(1, 23) //base level numbers
var numbers = initializeNumbers(grabQuestion)

val operations = determineOperations(level)