Dice roller app, formatting attribute id calls using Kotlin-CodePudding

I'm taking a kotlin basics course from the android devs website and we have to create an app that allows you to roll a dice and display an image with the number that rolled. I can make a condition for each possible roll number, but i feel like that's dumb and with formatted reference id calls it should be done way better but I don't know how to accomplish that in kotlin using Android Studio. Any helpers?

package com.example.diceroller

import android.os.Bundle
import android.widget.Button
import android.widget.ImageView
import androidx.appcompat.app.AppCompatActivity

/**
 * Calls the onCreate() method to initiate the app in its MainActivity.
 * setOnClickListener to look for activity in the window (button presses/taps).
 */

class MainActivity : AppCompatActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_main)

        val rollButton: Button = findViewById(R.id.button)
        rollButton.setOnClickListener { rollDice() }
        /* Toast.makeText(this,"Dice Rolled!", Toast.LENGTH_SHORT).show() | Shows alert (toast) at bottom of the screen. */
    }

    /*
    * Initiates a 'Dice' instance with 6 sides.
    * calls the roll() method to call rng between 1 and numSides.
    *
     */
    private fun rollDice() {
        val dice = Dice(6)
        val diceRoll = dice.roll()
        val diceImage: ImageView = findViewById(R.id.imageView)
        if (diceRoll == 1){ diceImage.setImageResource(R.drawable.dice_1) }
        else if (diceRoll == 2){ diceImage.setImageResource(R.drawable.dice_2) }
        //Instead of doing it the "dumb way" I want to call the id depending on diceRoll value.
    }
}

class Dice(private val numSides: Int) {
    fun roll(): Int {
        return (1..numSides).random()
    }
}

CodePudding user response：

If you're asking how to add 1 or whatever to "R.drawable.dice_" and then turn that string into a resource ID, then you can follow the accepted answer here: https://stackoverflow.com/a/2414165/13598222

Personally I wouldn't recommend that - it's brittle since the compiler can't connect what you're doing to a specific resource. So it can't warn you if you change the resource name (so your code doesn't match anymore), and things like Proguard can end up stripping out "unused" resources (because it can't tell you're using them) and you have to manually fix that.

Probably the most standard way to do what you're doing is to use a when clause, and just return the drawable ID from that:

val drawableId = when(diceRoll) {
    1 -> R.drawable.dice_1
    2 -> R.drawable.dice_2
    3 -> R.drawable.dice_3
    4 -> R.drawable.dice_4
    5 -> R.drawable.dice_5
    else -> R.drawable.dice_6
}
diceImage.setImageResource(drawableId)

Sure it's a little repetitive, but it's a standard pattern, it's very easy to understand and work with, and you're explicitly handling all the cases by adding the (required) else clause - you could also handle 6 explicitly and make the else throw an out-of-range message, in case your roll function somehow sends an invalid value (which it could, since you're hardcoding six values here, but your Dice class can have an arbitrary number!)

Another way you could do it is to have a lookup, like a Map:

private val diceDrawables = mapOf {
    1 to R.drawable.dice_1,
    2 to R.drawable.dice_2,
    3 to R.drawable.dice_3,
    4 to R.drawable.dice_4,
    5 to R.drawable.dice_5,
    6 to R.drawable.dice_6
}

and then your image setting code can be nice and short:

diceImage.setImageResource(diceDrawables[diceRoll] ?: throw Exception("No image for roll: $diceRoll))

This isn't the "dumb" way to do things, it's neat and tidy and avoids repetition (e.g. repeating the setImageResource call). Getting clever with your code logic can cut down a few lines, but it can also make things much more complex and fragile, and just harder to work with. Sometimes that kind of thing is the right call, but in this situation I think this is probably the best compromise. Your call though!

CodePudding user response：

Create a list using those resource ids.

private fun rollDice() {
    val diceImages = listOf(R.drawable.dice_1, R.drawable.dice_2, ...)
    val diceImage: ImageView = findViewById(R.id.imageView)
    discImage.setImageResource(discImages.random())
}