New to Elixir - Mind reviewing my code?

I’m new to Elixir and just recently completed “Elixir for Programmers” by pragdave.

I thought creating tic tac toe in Elixir would be a good next step for me to continue my learning.

I’ve created the logic for the tic tac toe game and was wondering if anyone would mind reviewing it and providing me with feedback?

The meat of the code starts from here:

A couple of things I’d like to see if can be improved:

Thanks very much!

3 Likes

First thing first, always run your code through the elixir formatter. :slight_smile:

The way you are doing it is fine:

    game = Map.put(game, :board, new_board)
    |> Map.put(:whos_turn, rotate_player(who))
    Map.put(game, :game_state, update_game_state(game))

Though more efficiently would probably be (assuming you can write update_game_state to work on the old state):

  defp accept_move(game = %Game{}, who, index, true) do
    %{game |
      board: put_elem(game.board, index, who),
      whos_turn: rotate_player(who),
      game_state: update_game_state(game),
    }
  end

Use not in:

  def check_board({a, b, c, d, e, f, g, h, i}) when nil not in [a, b, c, d, e, f, g, h, u] do
    :draw
  end
  def check_board(no_nils), do: :in_progress

Otherwise there are a few places in code that could be simplified more but I’m a bit busy at moment… ^.^;

7 Likes

I’d drop pattern matching and do something like this in the end

def check_board(board) do
  if Enum.count(board, &(&1 != nil)) == 0 do
    :draw
  else
    :in_progress
  end
end

upd it’d be wrong though since tuples are not enumerable in elixir, sorry for the confusion :smiley:

1 Like

You can always Tuple.to_list/1 them, but the pattern matching way is probably a lot more efficient I’d wager, if that even matters. :slight_smile:

1 Like

A really small note about aliasing… You can use MODULE when aliasing the module.

eg.

defmodule Tictactoe.Game do
  alias __MODULE__
end
6 Likes

Thanks very much @OvermindDL1 @yurko and @kokolegorille !! Great feedback.

4 Likes