4
\$\begingroup\$

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

image1

image2

image3

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.

\$\endgroup\$
2
  • \$\begingroup\$ By the way, you can change the checkmark to forsvarir's answer. His answer is much more useful than mine. \$\endgroup\$ May 9, 2021 at 0:30
  • \$\begingroup\$ @StephanBranczyk Anyway: Thanks a lot. \$\endgroup\$ May 9, 2021 at 5:15

2 Answers 2

3
\$\begingroup\$

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()
}
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ private fields/functions aren't accessible from outside of the class they are declared in. By having them private you help to enforce the encapsulation of the class, so for example you know clients will call the public methods to change the game state (and anything related) rather than risking a client updating the game state without this being reflected in the UI. \$\endgroup\$
    – forsvarir
    May 8, 2021 at 16:57
  • \$\begingroup\$ @forsvarir, Ok, I see. I'll need to start integrating that habit into my own coding. Thank you. \$\endgroup\$ May 9, 2021 at 0:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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