Better?

# alias IslandsEngine.{Game, Rules}
# {:ok, game} = Game.start_link("Miles")
# Game.guess_coordinate(game, :player1, 1, 1)
# Game.add_player(game, "Trane")
# Game.position_island(game, :player1, :dot, 1, 1)
# Game.position_island(game, :player2, :square, 1, 1)
# state_data = :sys.get_state(game)
# state_data = :sys.replace_state(game, fn data -> %{state_data | rules: %Rules{state: :player1_turn}} end)
# state_data.rules.state
# Game.guess_coordinate(game, :player1, 5, 5)
# Game.guess_coordinate(game, :player1, 3, 1)
# Game.guess_coordinate(game, :player2, 1, 1)

defmodule IslandsEngine.State do
  alias IslandsEngine.{Board, Coordinate, Guesses, Island, Rules}

  @players [:player1, :player2]

  def init(name) do
    player1 = %{name: name, board: Board.new(), guesses: Guesses.new()}
    player2 = %{name: nil, board: Board.new(), guesses: Guesses.new()}
    %{player1: player1, player2: player2, rules: %Rules{}}
  end

  def add_player(state, name) do
    with {:ok, rules} <- Rules.check(state.rules, :add_player) do
      state
      |> update_player2_name(name)
      |> update_rules(rules)
      |> success()
    else
      :error -> :error
    end
  end

  def position_island(state, player, key, row, col) when player in @players do
    board = player_board(state, player)

    with {:ok, rules} <-
           Rules.check(state.rules, {:position_islands, player}),
         {:ok, coordinate} <-
           Coordinate.new(row, col),
         {:ok, island} <-
           Island.new(key, coordinate),
         %{} = board <-
           Board.position_island(board, key, island) do
      state
      |> update_board(player, board)
      |> update_rules(rules)
      |> success()
    else
      error -> error
    end
  end

  def set_islands(state, player) when player in @players do
    board = player_board(state, player)

    with {:ok, rules} <- Rules.check(state.rules, {:set_islands, player}),
         true <- Board.all_islands_positioned?(board) do
      state
      |> update_rules(rules)
      |> success({:ok, board})
    else
      :error -> :error
      false -> {:error, :not_all_islands_positioned}
    end
  end

  def guess_coordinate(state, player, row, col) when player in @players do
    opponent_key = opponent(player)
    opponent_board = player_board(state, opponent_key)

    with {:ok, rules} <-
           Rules.check(state.rules, {:guess_coordinate, player}),
         {:ok, coordinate} <-
           Coordinate.new(row, col),
         {hit_or_miss, forested_island, win_status, opponent_board} <-
           Board.guess(opponent_board, coordinate),
         {:ok, rules} <-
           Rules.check(rules, {:win_check, win_status}) do
      state
      |> update_board(opponent_key, opponent_board)
      |> update_guesses(player, hit_or_miss, coordinate)
      |> update_rules(rules)
      |> success({hit_or_miss, forested_island, win_status})
    else
      error ->
        error
    end
  end

  defp player_board(state, player),
    do: Map.get(state, player).board

  defp opponent(:player1),
    do: :player2

  defp opponent(:player2),
    do: :player1

  defp update_rules(state, rules),
    do: %{state | rules: rules}

  defp update_player2_name(state, name),
    do: put_in(state.player2.name, name)

  defp update_board(state, player, board),
    do: Map.update!(state, player, fn player -> %{player | board: board} end)

  defp update_guesses(state, player, hit_or_miss, coordinate) do
    update_in(state[player].guesses, fn guesses ->
      Guesses.add(guesses, hit_or_miss, coordinate)
    end)
  end

  defp success(state),
    do: {:ok, state}

  defp success(state, other),
    do: {:ok, state, other}
end

defmodule IslandsEngine.Game do
  use GenServer

  alias IslandsEngine.State

  ## --- API
  @players [:player1, :player2]

  def add_player(game, name) when is_binary(name),
    do: GenServer.call(game, {:add_player, name})

  def position_island(game, player, key, row, col) when player in @players,
    do: GenServer.call(game, {:position_island, player, key, row, col})

  def set_islands(game, player) when player in @players,
    do: GenServer.call(game, {:set_islands, player})

  def guess_coordinate(game, player, row, col) when player in @players,
    do: GenServer.call(game, {:guess_coordinate, player, row, col})

  ## ---

  def via_tuple(name), 
    do: {:via, Registry, {Registry.Game, name}}

  def start_link(name) when is_binary(name),
    do: GenServer.start_link(__MODULE__, name, name: via_tuple(name))

  def init(name),
    do: {:ok, State.init(name)}

  def handle_call({:add_player, name}, _from, state) do
    case State.add_player(state, name) do
      {:ok, next_state} ->
        reply_success(next_state, :ok)

      error ->
        {:reply, error, state}
    end
  end

  def handle_call({:position_island, player, key, row, col}, _from, state) do
    case State.position_island(state, player, key, row, col) do
      {:ok, next_state} ->
        reply_success(next_state, :ok)

      error ->
        {:reply, error, state}
    end
  end

  def handle_call({:set_islands, player}, _from, state) do
    case State.set_islands(state, player) do
      {:ok, next_state, reply} ->
        reply_success(next_state, reply)

      error ->
        {:reply, error, state}
    end
  end

  def handle_call({:guess_coordinate, player, row, col}, _from, state) do
    case State.guess_coordinate(state, player, row, col) do
      {:ok, next_state, reply} ->
        reply_success(next_state, reply)

      error ->
        {:reply, error, state}
    end
  end

  defp reply_success(state, reply), do: {:reply, reply, state}
end

Now State contains the logic/core (and can be tested separately) - Game has been reduced to a process (GenServer) shell.

My first impression here is that I’m wary of trying to apply ideas from other ecosystems wholesale onto the BEAM ecosystem. When I was new to Elixir, I did it, and I know it did not help me.

Part of the idea of the book is to explore what’s possible in this new world. There are things that we can do on the BEAM that you would be hard pressed to do in other languages/ecosystems. Trying to apply rules/approaches/schemes from those other ecosystems onto this one seems unhelpful.

My strong preference would be to look at what we’re doing in the book and judge it on it’s own merits rather than comparing it to other systems or trying to fit it into a paradigm from another ecosystem.

That’s me trying to say, let’s please leave Java at the door. :^)

I’m also reminded of that old joke about the best answers to software questions always beginning with, “It depends.”

To me, questions like the one we’re looking at come into the realm of design decisions based on the constraints of the individual project. As we have no doubt seen, the only constant in software work is change.

The most important thing, then, is to design things with flexibility in mind. The individual choices we make are important, but the ability to easily change what we’ve done seems to me to be even more important.

Early in the book, we talk about having the ability to run thousands and thousands of games on the same node. I take that a constraint on the system. We need to solve for that in the design.

You’ve agreed that when we build a GenServer, we’re really defining a module and functions that work on some data. The change is that we decided that those functions should all run in a separate process (or processes).

To me, that says that we’re still working with the same things we were working with when we defined Board, Island, Guesses, and Coordinate, but by making this piece a GenServer, we satisfy the need for many games on the same node.

By using GenServer, we just did the two things at once - define the entity with a module, some functions, and some data as well as allow us to run it in multiple processes.

Here’s where your personal choice comes in. If this makes you feel uncomfortable, you can absolutely use the code for an intermediary module that @peerreynders wrote up. All will be well.

To me, the key point is that the design we have in the book makes that refactor a straight forward affair. You could do it wholesale. You could do it one api function/callback at a time.

It’s your choice. You can do it in response to changing requirements. You can manage the transition safely.

Is there a “shortage” of Erlang/Elixir patterns? Maybe, maybe not:

Garrett Smith attempted an Erlang specific repository but contributions stalled.

And even if a prescriptive pattern was followed - it could still be wrong if it is applied in the wrong context (this is a real problem in OOP with respect to design patterns).

Ultimately Game is just a process manipulating some data structures (Board, Coordinate, Guesses, Island, Rules) - and Game itself is going to be part of a supervision tree which itself is part of an OTP application.

For a discussion on how to structure supervision trees: The Hitchhiker’s Guide to the Unexpected
Data structure vs Process: To spawn, or not to spawn?
Structured Programming Using Processes (2004)

Building applications in INSERT_TECHNOLOGY_X_HERE doesn’t change how good applications are built

Yes, but going from Java to C# isn’t changing the paradigm so the solution shape isn’t impacted. But going to Erlang/Elixir you are shifting paradigms into “Concurrency Oriented Programming” which means that processes and most of OTP becomes part of the Core rather than being a

What seems to currently be challenging you the most is that a process can be an entity.

I’ve commented on the onion architecture before and in my view the most insightful aspect was that the domain layer gets to dictate the contracts while the infrastructure has to implement those contracts (e.g. nothing in the data store is influencing what is going on in the domain).

That particular advice still applies to Elixir - if that is the type of system that is being built.

Most of these architectures boil down to examining the structure and characteristics of a system along certain dimensions:

• Boundaries
  • at all levels of granularity (types, user defined types (modules), processes, supervision trees, applications, systems)
  • physical vs logical boundaries
  • (high) cohesion of logic within any one boundary
  • (low) coupling between boundaries
    • dependencies from within the boundary
    • dependencies on the boundary
    • i.e. good inter-boundary contracts (hiding implementation details, correct (specification → implementation) direction, etc.)

Alongs those lines what are the problems that you see with the design of Game?

Now there are some games you can play to hide whether or not something is a data structure or a process. But processes that are part of a supervision tree are known to be processes - otherwise they couldn’t be part of the tree.