FunctionClauseError is what I’d expect from iteraterefs if it was called with number < divisor. You likely need to define it for those inputs as well to pass Exercism’s tests.

General note: this isn’t going to do what you want. i = i + 1 rebinds the name i inside the do / end block but that value doesn’t escape or even make it to the next iteration.

Same thing for out = out <> elem(table, i) inside iteraterefs; the name out is bound to a new name but then the scope ends immediately.

My recommendation would be to forget completely about Enum.each for a little while; it is almost never the right solution in Elixir.

I went away, far up into the mountains which are the Elixir documents, I learned many things and when I returned to take on the challenge again, it neatly folded before me like the recursion I used to solve it:

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table {"M", "CM", "D", "CD", "C", "XC", "L", "XL", "X", "IX", "V", "IV", "I"}
  @refs {1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1}
  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @refs, @table, 0, "")
  end

  defp iterate(num, refs, table, index, out) do
    if num == 0 do 
      out
    else
      ref = elem(refs, index)
      cond do
        num >= ref -> sym = elem(table, index)
                      iterate(num - ref, refs, table, index, out <> sym)
        true -> iterate(num, refs, table, index + 1, out)
      end
    end
  end 
end

Nicely done, a big improvement! Probably the only other change I’d advocate for is to do this:

  defp iterate(0, _refs, _table, _index, out) do
    out
  end

  defp iterate(num, refs, table, index, out) do
    ref = elem(refs, index)
    cond do
      num >= ref ->
        sym = elem(table, index)
        iterate(num - ref, refs, table, index, out <> sym)
      true ->
        iterate(num, refs, table, index + 1, out)
    end
  end

It’s conventional that when you’re doing recursion like this to define the “base case” or “termination case” as its own clause up front, and then you have other clauses after. This is entirely a stylistic thing though, so it’s up to you!

Looks reasonable, though using lists can help avoid tricky off-by-one or off-the-end issues with elem.

As a demonstration of that principle, here are some refactors of the module:

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table {{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}}

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, 0, "")
  end

  defp iterate(num, table, index, out) do
    if num == 0 do 
      out
    else
      {sym, ref} = elem(table, index)
      cond do
        num >= ref ->
          iterate(num - ref, table, index, out <> sym)

        true ->
          iterate(num, table, index + 1, out)
      end
    end
  end
end

This first refactor keeps the tuple-shaped table, but brings the symbols and the values together for readability and future maintainability. Having @table and @refs be different-sized tuples would be Not Good, so combining them together makes that bug impossible.

There’s another place for bugs to hide, though: when we write iterate(num, table, index+1, out), that will try to execute elem(table, index+1) and give:

** (ArgumentError) errors were found at the given arguments:

  * 1st argument: out of range

    :erlang.element(14, {{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}})
    iex:21: RomanNumerals.iterate/4

(this happened in an earlier version when I made a typo)

The root cause is that the recursive call to iterate assumes that a “next” element of table exists, without checking. You could add extra checks to ensure that index < tuple_size(table), but again it’s better to write code that literally can’t run off the end.

First, a very bad refactor that makes things slower. Enum.at is expensive compared to elem, since it needs to traverse the list. This also applies the early-exit cleanup that @benwilson512 suggested.

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, 0, "")
  end

  defp iterate(0, _, _, out), do: out

  defp iterate(num, table, index, out) do
    {sym, ref} = Enum.at(table, index)
    cond do
      num >= ref ->
        iterate(num - ref, table, index, out <> sym)

      true ->
        iterate(num, table, index + 1, out)
    end
  end
end

This will fail in a slightly different way for an negative input (match error vs argument error) but it still “goes off the end” and crashes. Checking length(table) is expensive (traversing the list again!) so checking is even harder. Why am I telling you to use lists anyways?

Two things come together to make lists powerful here:

• a call to iterate will only ever care about index or higher in table

• There’s one place in a list that isn’t expensive to access: the first element (aka “the head”). It’s also easy to check for, since only [] doesn’t have a head.

This refactor applies that principle: instead of keeping track of table and index separately, it uses hd and tl to interact with the first element and the rest. This is fast and requires no allocations.

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, "")
  end

  defp iterate(0, _, out), do: out

  defp iterate(num, table, out) do
    {sym, ref} = hd(table)
    cond do
      num >= ref ->
        iterate(num - ref, table, out <> sym)

      true ->
        iterate(num, tl(table), out)
    end
  end
end

A small cleanup refactor: the pattern of “do something with hd and something else with tl” is so common that it’s usually not written explicitly. This uses pattern-matching to accomplish the same thing as the previous version

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, "")
  end

  defp iterate(0, _, out), do: out

  defp iterate(num, [head | rest] = table, out) do
    {sym, ref} = head
    cond do
      num >= ref ->
        iterate(num - ref, table, out <> sym)

      true ->
        iterate(num, rest, out)
    end
  end
end

It’s also very common to absorb a binding like {sym, ref} = head into the function head:

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, "")
  end

  defp iterate(0, _, out), do: out

  defp iterate(num, [{sym, ref} | rest] = table, out) do
    cond do
      num >= ref ->
        iterate(num - ref, table, out <> sym)

      true ->
        iterate(num, rest, out)
    end
  end
end

This version crashes in yet a different way, so what can we do about it? The error message gives us a clue:

iex(26)> RomanNumerals.numeral(-1)                                                                                                                                       
** (FunctionClauseError) no function clause matching in RomanNumerals.iterate/3    
    
    The following arguments were given to RomanNumerals.iterate/3:
    
        # 1
        -1
    
        # 2
        []
    
        # 3 
        ""
    
    iex:36: RomanNumerals.iterate/3

Think about what iterate being called with [] for table means: the conversion process has run out of symbols to try. That’s directly representable in code:

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, "")
  end

  defp iterate(0, _, out), do: out
  defp iterate(_, [], _), do: raise("bad number")

  defp iterate(num, [{sym, ref} | rest] = table, out) do
    cond do
      num >= ref ->
        iterate(num - ref, table, out <> sym)

      true ->
        iterate(num, rest, out)
    end
  end
end

Another not-actually-relevant-here-but-worth-keeping-in-mind tip on performance: <> in a loop (or recursion) should be regarded with suspicion as it can result in lots of short-lived binaries. A common idiom avoids the repeated operations and does them at the end:

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, [])
  end

  defp iterate(0, _, out), do: out |> Enum.reverse() |> Enum.join()
  defp iterate(_, [], _), do: raise("bad number")

  defp iterate(num, [{sym, ref} | rest] = table, out) do
    cond do
      num >= ref ->
        iterate(num - ref, table, [sym | out])

      true ->
        iterate(num, rest, out)
    end
  end
end

As a final cleanup, the cond with one non-default branch could be replaced with an if - or even a guard! The guard style makes the control structures disappear almost completely:

defmodule RomanNumerals do
  @doc """
  Convert the number to a roman number.
  """
  @table [{"M", 1000}, {"CM", 900}, {"D", 500}, {"CD", 400}, {"C", 100}, {"XC", 90}, {"L", 50}, {"XL", 40}, {"X", 10}, {"IX", 9}, {"V", 5}, {"IV", 4}, {"I", 1}]

  @spec numeral(pos_integer) :: String.t()
  def numeral(number) do
    iterate(number, @table, [])
  end

  defp iterate(0, _, out), do: out |> Enum.reverse() |> Enum.join()
  defp iterate(_, [], _), do: raise("bad number")

  defp iterate(num, [{sym, ref} | _] = table, out) when num >= ref do
    iterate(num - ref, table, [sym | out])
  end

  defp iterate(num, [_ | rest], out) do
    iterate(num, rest, out)
  end
end

Apologies for the long post, but I wanted to avoid the usual “the steps between these two versions are OBVIOUS” hand-waving and justify each piece.

Pinning variables only performs a match, it doesn’t create a binding. You aren’t actually doing any pinning in your example so I’m not quite sure how you are picturing it would work but, for example, this doesn’t work:

foo = 1
^foo = 1 + 1 # This is a match error

^foo = 1 + 1 gets expanded to 1 = 2 which, of course, does not match.

If you want to accumulate a variable the most basic ways to are either recursion or Enum.reduce.