-
-
Notifications
You must be signed in to change notification settings - Fork 400
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New practice exercise state-of-tic-tac-toe
#1115
New practice exercise state-of-tic-tac-toe
#1115
Conversation
Thank you for contributing to Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:
Automated comment created by PR Commenter 🤖. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was really enjoying this one until the last test (not your fault) 😞
exercises/practice/state-of-tic-tac-toe/lib/state_of_tic_tac_toe.ex
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,40 @@ | |||
defmodule StateOfTicTacToe do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My solution (without looking at yours first)
defmodule StateOfTicTacToe do
@symbol_map %{
"X" => :x,
"O" => :o
}
@doc """
Determine the state a game of tic-tac-toe where X starts.
"""
@spec gamestate(board :: String.t()) :: {:ok, :win | :ongoing | :draw} | {:error, String.t()}
def gamestate(board) do
board
|> parse_board()
|> check_state()
end
defp parse_board(board) do
board
|> String.split("\n", trim: true)
|> Enum.map(fn row ->
String.split(row, "", trim: true)
|> Enum.map(&@symbol_map[&1])
end)
end
defp check_state(parsed_board) do
case validate_state(parsed_board) do
:ok -> find_winners(parsed_board)
{:error, error} -> {:error, error}
end
end
defp frequencies(parsed_board) do
plays = parsed_board |> List.flatten() |> Enum.filter(& &1)
Map.merge(%{x: 0, o: 0}, Enum.frequencies(plays))
end
defp validate_state(parsed_board) do
frequencies = frequencies(parsed_board)
cond do
frequencies.o > frequencies.x -> {:error, "Wrong turn order: O started"}
frequencies.x > frequencies.o + 1 -> {:error, "Wrong turn order: X went twice"}
true -> :ok
end
end
defp find_winners(parsed_board) do
full_board? = parsed_board |> List.flatten() |> Enum.reject(& &1) |> Kernel.==([])
row_winners = row_winners(parsed_board)
column_winners = column_winners(parsed_board)
diagonal_winners = diagonal_winners(parsed_board)
winners =
(column_winners ++ row_winners ++ diagonal_winners)
|> Enum.filter(& &1)
|> Enum.uniq()
cond do
winners == [] && full_board? ->
{:ok, :draw}
winners == [] ->
{:ok, :ongoing}
length(winners) == 1 ->
{:ok, :win}
length(winners) == 2 ->
symbol = find_who_should_have_won(parsed_board)
{:error, "Impossible board: game should have ended after #{symbol} won"}
end
end
defp row_winners(parsed_board) do
parsed_board
|> Enum.map(&winner(&1))
end
defp column_winners(parsed_board) do
parsed_board
|> Enum.zip()
|> Enum.map(&winner(Tuple.to_list(&1)))
end
defp diagonal_winners(parsed_board) do
down_right =
parsed_board
|> Enum.with_index()
|> Enum.map(fn {_, i} -> Enum.at(Enum.at(parsed_board, i), i) end)
up_right =
parsed_board
|> Enum.with_index()
|> Enum.map(fn {_, i} -> Enum.at(Enum.at(parsed_board, length(parsed_board) - i - 1), i) end)
[winner(down_right), winner(up_right)]
end
defp winner(list) do
if length(Enum.uniq(list)) == 1, do: hd(list), else: nil
end
defp find_who_should_have_won(parsed_board) do
frequencies = frequencies(parsed_board)
# this is not technically true because it is impossible to say who should have won in certain cases
# without knowing the actual play older, e.g.:
#
# XXX
# OOO
# X..
#
# In this case, the winner depends on what was X's last move.
# If it was in the first column bottom row, then X won first.
# If it was in last column first row, then O won first.
should_have_won =
cond do
frequencies.x <= frequencies.o -> :x
frequencies.x > frequencies.o -> :o
end
reverted_symbol_map = Map.new(@symbol_map, fn {key, val} -> {val, key} end)
reverted_symbol_map[should_have_won]
end
end
@tag :pending | ||
test "Invalid board (3)" do | ||
board = """ | ||
XXX | ||
OOO | ||
... | ||
""" | ||
|
||
assert StateOfTicTacToe.gamestate(board) == | ||
{:error, "Impossible board: game should have ended after X won"} | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of this test. It forces you to handle a case of two winners, but more detailed tests cases are lacking because it's impossible to handle two winners correctly in all cases. It felt wrong writing code handling this... I even left a comment in my solution. Basically in this case, assuming the right round order was followed, it's clear that X won because X started. But in this case, it's no longer clear:
XXX
OOO
X..
The real winner depends on which X was played in which order. If the bottom row X was last, then X won. But it wasn't last, then O won.
I wish this corner case of "two winners" was not even touched in the problem specs but I'm not sure if it makes sense to argue. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see your point, I think you're right.
Here is what I would suggest:
Change the error message from "Impossible board: game should have ended after X won" to "Impossible board: game should have ended after the game was won", and also add your test case with the same message.
Shall I go ahead and change it here and then do a PR on problem-specs?
The exercise is very young, so this is valuable input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was really enjoying this one until the last test (not your fault) 😞
Sorry! 😅
@jiegillet I like that change, it mirrors how I expected people to solve it (counting the number of wins, not caring about the order)
Co-authored-by: Angelika Tyborska <angelikatyborska@fastmail.com>
Awesome 👏 |
This is somewhat nostalgic :)