Elixir for programmers: how would you manage this changing of state?

Questions / Help
,
1

Hello :slight_smile:

I’m learning Elixir by following this course by Dave Thomas, here: The Coding Gnome

Which is great btw.

However, I’m a little puzzled by a couple of aspects to which I’l like feedback from experienced Elixir programmers.

In this course, we program a “hangman” game.

From a high level perspective, the consuming client can initialize a new_game(), and update the internal state as a side effect by calling consecutive make_move(String.t) until the game is finished.

iex(1)> game = Hangman.new_game()
%Hangman.Impl.Game{
  game_state: :initializing,
  letters: ["d", "r", "y", "e", "r"],
  turns_left: 7,
  used: #MapSet<[]>
}

iex(2)> Hangman.make_move(game, "d")
{%Hangman.Impl.Game{
   game_state: :good_guess, 
   letters: ["d", "r", "y", "e", "r"],
   turns_left: 5,
   used: #MapSet<["d"]>
 }, %{game_state: :good_guess, letters: [], turns_left: 5, used: ["d"]}}

In order to handle the changing of the internal state, we create many small methods functions and make extensive use of pattern matching.

Here is a little (truncated) example to illustrate:

def make_move(game, guess) do
  accept_guess(game, guess, MapSet.member?(game.used, guess))
end

defp accept_guess(game, _guess, _already_used = true) do
  %{ game | game_state: :already_used }
end

The first thing I notice that slightly bugs me, and this technique is used many times, is that the caller make_move handles the game logic (here by calling MapSet.member?(game.used, guess) to feed accept_guess’s third parameter. As a result, accept_guess becomes a “dumb” function.

I would tend to do the opposite so I would like to know if this is a common idiom in Elixir. And if so, what is the rationale behind inverting the logic this way.

One of the reasons given by the author is that programming this way reduces branching. Instead, we only add functions and use pattern matching: no if statements!

However, and this is my second question, I notice that the code becomes less readable or understandable to me. Because, as a result, the managing of state is then spread out to many functions and it’s hard to see at a glance what’s going on, what the real logic is, from top down.

Putting it into other words, the flow of the code becomes as follows:

Step 1: create a `new_game`
Step 2: take a turn and change the internal state with `make_move(String.t)`

Step 2 is represented by:

- `make_move` variant 1
- `make_move` variant 2
- `accept_guess` variant 1, called by `make_move`, sometimes
- `accept_guess` variant 2, called by `make_move`, sometimes
- `score_guess` variant 1, called by `accept_guess`, sometimes
- `score_guess` variant 2, called by `accept_guess`, sometimes
- `score_guess` variant 3, called by `accept_guess`, sometimes

I tried to reduce those many steps, and condense them into one function instead. As such:

defp update(game, guess) do
    duplicate_move? = MapSet.member?(game.used, guess)
    good_guess? = Enum.member?(game.letters, guess)
    new_used = MapSet.put(game.used, guess)

    {new_state, new_turns_left} =
      if(duplicate_move?) do
        {:already_used, game.turns_left}
      else
        if good_guess? do
          {:good_guess, game.turns_left - 1}
        else
          {:bad_guess, game.turns_left - 1}
        end
      end

    %{game | game_state: new_state, turns_left: new_turns_left, used: new_used}
  end

With this API, the calling code becomes more straightforward, resembling this (truncated to simplify the discussion)

def make_move(game, guess) do
  update(game, guess)
end

And the changing of the internal state (via update()) can be observed from one place in the code.

Although update is more complicated, I feel that it’s more readable and that the main logic can be understood at a glance.

Anyhow, I’d like to have your input on that: do you spread the changing of state to many functions or try to isolate it into one function as I did here and why?

If you do the latter, do you have any tips on how to handle this more “complicated” function better? I thought about introducing some kind of truth table looking code, as such:

defp update(game, guess) do
    duplicate_move? = MapSet.member?(game.used, guess)
    good_guess? = Enum.member?(game.letters, guess)
    new_used = MapSet.put(game.used, guess)

    {new_state, new_turns_left} =
      case {duplicate_move?, good_guess?} do
        {true, _} -> {:already_used, game.turns_left}
        {false, true} -> {:good_guess, game.turns_left - 1}
        {false, false} -> {:bad_guess, game.turns_left - 1}
      end

    %{game | game_state: new_state, turns_left: new_turns_left, used: new_used}
  end

Do you think it’s readable? Do you think of any way to express this code better?

Thanks for any input, I hope I’m clear enough ^^

4 Likes
2

I like your version of update more than the version above, but I think that this is just too small thing to reason about. Both variants are correct and readable and almost every Elixir developer has her/his own preference about some aspects of code

For example, I’ve seen that some people are strictly against piping into case (like |> case do), or some other people prefer to match in function arguments instead of case (which is just the thing you’re asking about)

So, don’t bother yourself with these things too much.

3 Likes
3

So initially when coming to Elixir, I was in the same camp that multiple function heads were less readable than conditional branching logic. I’ve since changed my mind. I think a large part of this was familiarity: every programming language I’d used over decades relied on if, case, switch, etc. so I’d come to read programs and conditional logic on that basis. This was overcome by habituation: writing and reading conditional logic in the form multiple function heads because more natural as my expectations changed. What I also realized is that if my functions were well named, that often reading any given function didn’t need me to dive into the details of the conditional logic unless I suspected that there was something I need to look at specifically.

This may be a little bit why such emphasis is placed on this in the @pragdave course. To start that process of getting you to see things in this more declarative expression that is common in Elixir programming. I’ve taken the course as well (both the old and the new) and was pleased that time was spent to break the reliance on old paradigms that I brought with me.

Whether over the long haul it matters a lot or not, as others have said, you’ll see varying opinions and certainly initially it’s not worth worrying too much about. But I would suggest give yourself time to acclimate before reaching a conclusion so that the conclusion is well considered and well informed.

2 Likes
4

IMO it’s Elixir’s equivalent of Haskell’s pointfree style - pattern-matching is powerful, but using it to do everything can obfuscate more than it helps.

My preference for complex code like this is to split things into successively smaller pieces while maintaining a consistent level of abstraction, ideally by identifying domain concepts and turning them into functions.

For instance, here’s a refactor of your update function:

defp update(game, guess) do
  cond do
    duplicate_move?(game, guess) ->
      already_used(game)

    good_guess?(game, guess) ->
      successful_choice(game, guess)

    true ->
      unsuccessful_choice(game, guess)
  end
end

defp already_used(game) do
  set_state(game, :already_used)
end

defp successful_choice(game, guess) do
  game
  |> apply_turn(guess)
  |> set_state(:good_guess)
end

defp unsuccesful_choice(game, guess) do
  game
  |> apply_turn(guess)
  |> set_state(:bad_guess)
end

defp duplicate_move?(game, guess) do
  MapSet.member?(game.used, guess)
end

defp good_guess?(game, guess) do
  Enum.member?(game.letters, guess)
end

defp set_state(game, state) do
  %{game | game_state: state}
end

defp apply_turn(game, guess) do
  %{game | turns_left: game.turns_left - 1, used: Mapset.put(game.used, guess)}
end

Some notes on the above:

  • almost every function in update takes game and guess as arguments; update does not “peek inside” the game struct directly
  • this is possibly too much splitting-apart for this level of code complexity; already_used, successful_choice and unsuccessful_choice could be inlined into the cond’s branches in update without obscuring things too much. I’ve split them out here for demonstration’s sake
  • the other four functions look suspiciously reusable (they make an API for interacting with Game structs), and also testable. Making them public functions on Game or a helper module could be useful
5 Likes
5

For sure it’s a small thing, and the code is correct yes :slight_smile:

But small little things add up so I’m okay to put thought into it sometimes.

That’s good to know thanks, I’ll keep that in mind.
I suppose I’ll have to abuse this technique to then find out where’s the sweet spot for me.

Thanks, that’s useful feedback. The cond construct looks great to me and the setter looking functions look like a great way to break things down too, I like it!

I suppose I was worried about what would happen with this technique once I pile on conditionals (as they do exist) and could see myself being lost trying to understand where the branching actually occurs or even not seeing a pattern emerge.

I could see testing becoming easier with this technique, but since we mostly talk about private functions here, that got me doubly confused. To me, update() is the kind of black box you can poke at from the outside, testing wise. And ok to be complicated as long as it does its job, being: “give me this input, and I’ll give you this output”.

Anyways, I’ll have to give this some more thought, thanks all :slight_smile:

1 Like