Text based tic-tac-toe in bash

Question

I wrote this two player tic-tac-toe program in bash. To make a move you enter a number between 0 and 8 which corresponds to the square:

0|1|2
3|4|5
6|7|8

This is my code:

#!/usr/bin/env bash

declare -r BOARD_SIZE=9
declare -r DEFAULT_SQUARE_VALUE=-
declare -ra PLAYERS=(O X)

draw_board() {
    for i in {1..3}
    do
        echo "${1:- }|${2:- }|${3:- }"
        shift 3
    done
}


get_placement() {
    while :
    do
        echo "Please enter a valid move (0-$((BOARD_SIZE - 1)))"
        read n
        echo

        digit_pattern="^[0-$((BOARD_SIZE-1))]\$"
        args=("$@")
        if [[ $n =~ $digit_pattern ]] && [[ ${args[$n]} == $DEFAULT_SQUARE_VALUE ]]
        then
            placement=$n
            return
        fi

    done
}

# sets $ended to whether the game has ended and $winner to the winner
get_winner() {
    ended=1
    possible_draw=1

    # check for draw
    for square in $@
    do
        if [[ $square == $DEFAULT_SQUARE_VALUE ]]
        then
            possible_draw=0
            break
        fi
    done

    # check for winners
    if [[ ${PLAYERS[*]} =~ "$1" ]] && [[ ${PLAYERS[*]} =~ "$5" ]] && [[ ${PLAYERS[*]} =~ "$9" ]]
    then
        if [[ "$1" == "$5"  ]] && [[ "$5" == "$9" ]]
        then
            winner=$1
            return
        fi
    fi

    if [[ ${PLAYERS[*]} =~ "$3" ]] && [[ ${PLAYERS[*]} =~ "$5" ]] && [[ ${PLAYERS[*]} =~ "$7" ]]
    then
        if [[ "$3" == "$5"  ]] && [[ "$5" == "$7" ]]
        then
            winner=$3
            return
        fi
    fi

    for i in {1..3}
    do
        if [[ ${PLAYERS[*]} =~ "$1" ]] && [[ ${PLAYERS[*]} =~ "$2" ]] && [[ ${PLAYERS[*]} =~ "$3" ]]
        then
            if [[ "$1" == "$2"  ]] && [[ "$2" == "$3" ]]
            then
                winner=$1
                return
            fi
        fi

        if [[ ${PLAYERS[*]} =~ "$1" ]] && [[ ${PLAYERS[*]} =~ "$4" ]] && [[ ${PLAYERS[*]} =~ "$7" ]]
        then
            if [[ "$1" == "$4"  ]] && [[ "$4" == "$7" ]]
            then
                winner=$1
                return
            fi
        fi

        shift 3
    done

    winner=''

    if [[ $possible_draw == 1 ]]
    then
        return
    fi

    ended=0

} 

board=( $(for i in $(seq 1 $BOARD_SIZE); do echo "$DEFAULT_SQUARE_VALUE"; done) )
current_turn="O"

while :
do
    draw_board ${board[@]}
    get_placement ${board[@]}
    board[$placement]=$current_turn
    get_winner ${board[@]}    

    if [[ "$ended" == 1 ]]
    then
        if [ -n "$winner" ]
        then
            echo "$winner has won!"
            echo
            draw_board ${board[@]}
        else
            echo "Draw!"
            echo
            draw_board ${board[@]}
        fi

        exit
    fi

    [[ $current_turn == ${PLAYERS[0]} ]] && current_turn=${PLAYERS[1]} || current_turn=${PLAYERS[0]}

    echo -en "\n\n\n"
done

Example game:

-|-|-
-|-|-
-|-|-
Please enter a valid move (0-8)
0




O|-|-
-|-|-
-|-|-
Please enter a valid move (0-8)
8




O|-|-
-|-|-
-|-|X
Please enter a valid move (0-8)
6




O|-|-
-|-|-
O|-|X
Please enter a valid move (0-8)
3




O|-|-
X|-|-
O|-|X
Please enter a valid move (0-8)
2




O|-|O
X|-|-
O|-|X
Please enter a valid move (0-8)
4




O|-|O
X|X|-
O|-|X
Please enter a valid move (0-8)
1

O has won!

O|O|O
X|X|-
O|-|X

I do not know much about bash, so any feedback would be much appreciated.

Answer 1

The intent here is that an engineer who is not a Bash expert should be able to readily maintain the code some months down the road.

I do not know much about bash

You might be that future maintenance engineer. Be kind to your future self.

And thank you for that nice shebang.

true

get_placement() {
    while :

Please don't do that.

The Bourne shell has a long and sordid history which involves various (almost) work-alikes. The special builtin : colon operator used to be important when writing scripts.

Nowadays, please just write while true.

It's not like we're going to fork off a /usr/bin/true child. The shebang explained that we're targeting a shell, bash, which according to $ type true offers true as a shell builtin. Using arcane punctuation makes it harder for engineers to google for relevant technical advice. Using words (true) lets polyglot engineers leverage their knowledge from other environments into this one.

shellcheck

Code review submissions should lint clean before being submitted. When writing shell code it is easy to produce a result, but it is a minor challenge to make the script correct, due to quoting concerns and other minutiae.

Lint this code, and follow the linter's advice, so it lints clean. 'Nuff said.

spurious default

I don't understand this line:

draw_board() {
        ...
        echo "${1:- }|${2:- }|${3:- }"

That is, why are we defaulting to SPACE?

It seems that only [XO-] will ever be passed in. I am reluctant to support defaulting which is never used, out of concern that it may "paper over" some code defect that may be injected in future.

char vs int; magic constant

        digit_pattern="^[0-$((BOARD_SIZE-1))]\$"

One can play tic-tac-toe on boards of various sizes, including 2×2, 3×3, 4×4, ....

The BOARD_SIZE constant is lovely and I thank you for it.

Here, the digit_pattern expression is tightly coupled to the ... = 9 assignment. It precludes using a larger board, as then we'd need to worry about multi-digit labels for the squares.

The key difficulty is that we're validating against string (really char) values, instead of against integer values. I am sad that we don't exit with an error code if this line encounters a "large" board size.

zero- vs one- origin

The review context showed a board with squares labeled 0 .. 8, which is very helpful. I'm surprised the game never displays that to the player.

Consider using 1 .. 9 instead, even if that means allocating storage for an unused zeroth square. Then interpreting a bash expression like $8 would be more straightforward, it would have a more obvious correspondence to a certain square.

side effect comments

# sets $ended to whether the game has ended and $winner to the winner
get_winner() {

Thank you for explaining a non-obvious part of the contract, it's very helpful.

This function is Too Long, as one cannot read all of it at once, it needs vertical scrolling to review it. You could e.g. easily Extract Helper by writing a check_for_winner function.

On which topic, the check is entirely too tedious. Better to (roughly) loop over 159 357 123 147.

Also, the vertical check is incorrect, leading to prompts like this:

-|O|X
-|O|X
-|O|X
Please enter a valid move (0-8)

The shift 3 doesn't interact with the column check as you wanted it to. The row check works fine.

test suite

There isn't one, and you clearly need it, given the column check code defect.

Write down some example move sequences, and the expected winner outcome and board state. Automating this will allow you to view Green bar before each commit.

decomposition

I will point out that this is just beautiful:

do
    draw_board ${board[@]}
    get_placement ${board[@]}
    board[$placement]=$current_turn
    get_winner ${board[@]}

Nice names, very clear narrative, looks great.

UX

I found it a bit surprising that the prompt doesn't mention whether it's currently the turn of O or X. It's not necessarily obvious, since an invalid input of e.g. 9 keeps current player the same.

I was surprised that CTRL/D didn't exit the game due to EOF. An interactive player can always resort to CTRL/C. But automated unit tests may want to use redirection from an example game transcript file, and so reliably terminating on EOF might become an important requirement.

---

This code only achieves a subset of its design goals.

I would not be willing to delegate or accept maintenance tasks on it in its current form.