Android Tic-Tac-Toe

Question

I have made an Android Tic-Tac-Toe adaptation as a weekend-project.

Here's the Kotlin-code.

MainActivity:

package com.mizech.tictactoe

import android.content.DialogInterface
import android.content.Intent
import android.graphics.Color
import android.os.Bundle
import android.view.View
import android.widget.ImageView
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.app.AppCompatActivity
import com.google.android.material.snackbar.Snackbar
import com.mizech.tictactoe.databinding.ActivityMainBinding

class MainActivity : AppCompatActivity() {
    private lateinit var binding: ActivityMainBinding
    private var gameState = mutableListOf<FeasibleState>()
    private var isPlayerOne = true
    private var fieldsUsed = 0
    private val imageViews = mutableListOf<ImageView>()

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        binding = ActivityMainBinding.inflate(layoutInflater)
        val view = binding.root
        setContentView(view)

        var i = 0
        while (i < 9) {
            gameState.add(FeasibleState.NOT_SET)
            i++
        }
        // Add the imageViews to the List
        imageViews.add(binding.imageView0)
        imageViews.add(binding.imageView1)
        imageViews.add(binding.imageView2)
        imageViews.add(binding.imageView3)
        imageViews.add(binding.imageView4)
        imageViews.add(binding.imageView5)
        imageViews.add(binding.imageView6)
        imageViews.add(binding.imageView7)
        imageViews.add(binding.imageView8)
        // Iterate the list and attach an Listener to each.
        imageViews.forEach {
            it.setOnClickListener {
                processStateChange(it)
            }
        }

        binding.resetGame.setOnClickListener {
            val intent = Intent(this, MainActivity::class.java)
            val snackbar = Snackbar.make(it, "Game Reset cancelled",
                    Snackbar.LENGTH_LONG)
            val dialog = AlertDialog.Builder(this)
            dialog.apply {
                setIcon(R.drawable.ic_baseline_priority_high_24)
                setTitle("Game Reset")
                setMessage("Do you want to continue?")
                setPositiveButton("Yes", DialogInterface.OnClickListener { dialog, which ->
                    startActivity(intent)
                })
                setNegativeButton("No", DialogInterface.OnClickListener { dialog, which ->
                    snackbar.show()
                })
            }.show()
        }
    }

    private fun checkGameState(currentPlayer: FeasibleState): Boolean {
        if (gameState[0] === currentPlayer && gameState[1] === currentPlayer
                && gameState[2] === currentPlayer) {
            return true
        }

        if (gameState[3] === currentPlayer && gameState[4] === currentPlayer
                && gameState[5] === currentPlayer) {
            return true
        }

        if (gameState[6] === currentPlayer && gameState[7] === currentPlayer
                && gameState[8] === currentPlayer) {
            return true
        }

        if (gameState[0] === currentPlayer && gameState[3] === currentPlayer
                && gameState[6] === currentPlayer) {
            return true
        }

        if (gameState[1] === currentPlayer && gameState[4] === currentPlayer
                && gameState[7] === currentPlayer) {
            return true
        }

        if (gameState[2] === currentPlayer && gameState[5] === currentPlayer
                && gameState[8] === currentPlayer) {
            return true
        }

        if (gameState[0] === currentPlayer && gameState[4] === currentPlayer
                && gameState[8] === currentPlayer) {
            return true
        }

        if (gameState[2] === currentPlayer && gameState[4] === currentPlayer
                && gameState[6] === currentPlayer) {
            return true
        }

        return false
    }

    private fun processStateChange(it: View) {
        val imgView = it as ImageView
        if (isPlayerOne) {
            imgView.setImageResource(R.drawable.player_one)
            fieldsUsed++
            gameState[imgView.tag.toString().toInt()] = FeasibleState.PLAYER_ONE

            checkResult(FeasibleState.PLAYER_ONE, R.string.one_won_message, "#64FF64")
        } else {
            imgView.setImageResource(R.drawable.player_two)
            fieldsUsed++
            gameState[imgView.tag.toString().toInt()] = FeasibleState.PLAYER_TWO

            checkResult(FeasibleState.PLAYER_TWO, R.string.two_won_message, "#FF6464")
        }

        isPlayerOne = !isPlayerOne
        imgView.isEnabled = false
    }

    private fun checkResult(currentPlayer: FeasibleState, message: Int, winnerColor: String) {
        if (checkGameState(currentPlayer)) {
            setFinalResult(message, winnerColor)
        } else if (fieldsUsed == 9) {
            setFinalResult(R.string.tie_message, "#ff00ff")
        }
    }

    private fun setFinalResult(winnerString: Int, winnerColor: String) {
        imageViews.forEach {
            it.isEnabled = false
        }
        binding.currentMessage.text = getString(winnerString)
        binding.currentMessage.setTextColor(Color.parseColor(winnerColor))
    }
}

The enum-class:

package com.mizech.tictactoe

enum class FeasibleState {
  NOT_SET,
  PLAYER_ONE,
  PLAYER_TWO
}

The full source-code and additional images on GitHub.

How could my implementation become improved? What would you have done differently and why?

Is there a better possibility to reset the game? Instead of triggering an intent to MainActivity self.

Looking forward to reading your answers and comments.

Answer 1

Reset

Calling back into your MainActivity each time you reset the game creates a stack of the activities, so if the user plays several games and tries to quit by pressing the back button, they'll have to go through each of the previous games until they get out.

A better approach might be to have a reset method that you call after the user has confirmed, rather than starting a new activity. You essentially need to reset your game state and UI, which should look something like this:

private fun resetGame() {
    for (idx in 0..8) {
        gameState[idx] = FeasibleState.NOT_SET
    }
    imageViews.forEach { view ->
        view.setImageResource(R.drawable.not_set)
        view.isEnabled = true
    }
    binding.currentMessage.text = ""
    fieldsUsed = 0
    isPlayerOne = true
}

checkGameState

Your checkGameState function has a lot of duplicate lines, all of which are essentially doing the same thing. You are checking three game states to see if they match the current player. An alternative approach is to create a list of the valid winning combinations. You can then iterate through the list to check the combinations. Something like this:

private fun checkGameState(currentPlayer: FeasibleState): Boolean {
    val winningCombo = arrayOf(
        intArrayOf(0,1,2),
        intArrayOf(3,4,5),
        intArrayOf(6,7,8),
        intArrayOf(0,3,6),
        intArrayOf(1,4,7),
        intArrayOf(2,5,8),
        intArrayOf(0,4,8),
        intArrayOf(2,4,6)
    )

    return winningCombo.filter { it.all { idx-> gameState[idx] == currentPlayer }}.any()
}

Answer 2

Disclaimer: I'm far from being an expert, but since no one has responded to you yet and it's already been a couple of weeks. I thought I would try giving you some feedback.

Your function checkGameState contains a lot of duplicate code. Try using a helper function to get rid of that duplication.

gameState refers to the 9 squares of your grid. Maybe you should rename it to squares or to gridStates instead. Also, I don't think it needs to be mutable either. So I'd use an array and instead of writing this:

    private var gameState = mutableListOf<FeasibleState>()
    ...
    var i = 0
    while (i < 9) {
        gameState.add(FeasibleState.NOT_SET)
        i++
    }

I would declare and initialize this array both at the same time.

    var gridStates = Array(9, {FeasibleState.NOT_SET})

Now, this is probably me being nitpicky, but in my case, I would have used a loop to do imageViews.add(binding.imageViewx) and used an array to store those image names (or I would have generated the names with a string template). And once I had that loop, then I would have done the setting of the onClickListener within that same loop.