tio407

tio407

I have a hard time following the code at defp numeral function. Is this well written Elixir code or is it better to use pipes?

defmodule Roman do
  @numerals [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]

  @doc """
  Convert the number to a roman number.
  """
  @spec numerals(pos_integer) :: String.t()
  def numerals(number) do
    numeral([], number, @numerals)
  end

  defp numeral(curr_nums, number, [{letter, digit}|_t] = num_list) when number >= digit do
    numeral([letter | curr_nums], number - digit, num_list)
  end

  defp numeral(curr_nums, number, [_h|t]) do
    numeral(curr_nums, number, t)
  end

  defp numeral(numerals, _n, []) do
    numerals
      |> Enum.reverse
      |> Enum.map_join("", &to_string/1)
  end
end

What are some suggestions for re-writing this in a better way? It’s my old code but I no longer understand what is happening.

Showing Posts 1 to 3

kip

kip

ex_cldr Core Team

I think its fine except the variable naming makes it a bit hard to see what’s happening. And the odd comment helps. Personally I think a recursive function like this is a good fit for the problem. Perhaps like this:

defmodule Roman do
  @roman_to_decimal_mappings [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]

  @doc """
  Convert the number to a roman number.
  """
  @spec numerals(pos_integer) :: String.t()
  def numerals(number) do
    numeral([], number, @roman_to_decimal_mappings)
  end

  # While the number is >= the decimal value at the head of the mappings,
  # accumulate the Roman numeral and decrement the number
  defp numeral(roman_acc, number, [{roman_numeral, decimal_value}| _t] = roman_to_decimal_mappings) 
      when number >= decimal_value do
    numeral([roman_numeral | roman_acc], number - decimal_value, roman_to_decimal_mappings)
  end

  # When the number is less than the decimal value at the head of the
  # mapping list, move to the next mapping pair
  defp numeral(roman_acc, number, [_h | remaining_mappings]) do
    numeral(roman_acc, number, remaining_mappings)
  end

  # And when the mapping list is exhausted, reverse the accumulated
  # Roman numerals and join then
  defp numeral(roman_acc, _n, [] = _remaining_mappings) do
    roman_acc
      |> Enum.reverse
      |> Enum.map_join("", &to_string/1)
  end
end
gregvaughn

gregvaughn

I’m not certain how readable this is for others, but it’s how I worked through the problem early in my Elixir adventures, and I can still explain the approach 6 years later. elixir_kata/roman/roman_numerals.exs at master · gvaughn/elixir_kata · GitHub

I iterate over the mappings and use Enum.reduce with a 2-tuple to keep track of the remainder of the number (after dividing out each Roman value) plus the accumulated Roman numeral string.

collegeimprovements

collegeimprovements

defmodule RomanNumerals do
  @spec to_roman(integer) :: String.t()
  def to_roman(number) when is_integer(number) and number > 0 do
    to_roman_recursive(number, "")
  end
  def to_roman(_), do: ""

  defp to_roman_recursive(0, acc), do: acc
  defp to_roman_recursive(number, acc) when number >= 1000, do: to_roman_recursive(number - 1000, acc <> "M")
  defp to_roman_recursive(number, acc) when number >= 900, do: to_roman_recursive(number - 900, acc <> "CM")
  defp to_roman_recursive(number, acc) when number >= 500, do: to_roman_recursive(number - 500, acc <> "D")
  defp to_roman_recursive(number, acc) when number >= 400, do: to_roman_recursive(number - 400, acc <> "CD")
  defp to_roman_recursive(number, acc) when number >= 100, do: to_roman_recursive(number - 100, acc <> "C")
  defp to_roman_recursive(number, acc) when number >= 90, do: to_roman_recursive(number - 90, acc <> "XC")
  defp to_roman_recursive(number, acc) when number >= 50, do: to_roman_recursive(number - 50, acc <> "L")
  defp to_roman_recursive(number, acc) when number >= 40, do: to_roman_recursive(number - 40, acc <> "XL")
  defp to_roman_recursive(number, acc) when number >= 10, do: to_roman_recursive(number - 10, acc <> "X")
  defp to_roman_recursive(number, acc) when number >= 9, do: to_roman_recursive(number - 9, acc <> "IX")
  defp to_roman_recursive(number, acc) when number >= 5, do: to_roman_recursive(number - 5, acc <> "V")
  defp to_roman_recursive(number, acc) when number >= 4, do: to_roman_recursive(number - 4, acc <> "IV")
  defp to_roman_recursive(number, acc) when number >= 1, do: to_roman_recursive(number - 1, acc <> "I")
end
— All posts loaded —

Where Next? Top

Trending in Questions Top

Blokh
Hey guys, I’ve got a huge CSV ( around 10 GB ) that needs to be processed hourly Do you guys have any suggestions what is the best prac...
New
kszambelanczyk
Hello! Could someone please give me a help/sample code, how to delete a file from s3 using waffle/waffle_ecto from Phoenix app. I creat...
New
Onor.io
I have what I’ve heard referred to as a “lookup table” in my database. This is a way of assigning codes to common values. One common lo...
New
Trolleger
What approach to take when sending live updates to “random” users Hi! I have a question, I have a little chat app, and when I create a DM...
New
matt-savvy
Anyone here using Honeybadger? My Honeybadger account is being overwhelmed with noise from some bots. Seeing a lot of Bandit.HTTPError...
New
RemyXRenard
I’m seeing that a list inside a Kino.DataTable will be interpreted as a charlist, even if the Kino.configure() is set to charlists: :as_l...
New
samoloth
Hi, I’ve just set up an application with ash_authentication. There is only magic link strategy for now, so there is no confirmation add o...
New

Other Trending Topics Top

garrison
Hobbes is a low-level distributed database for the Elixir programming language. Hobbes provides a simple, safe, and scalable storage lay...
New
mcass19
ExRatatui lets you cook up rich terminal UIs in Elixir, powered by Rust’s ratatui via Rustler NIFs. Build interactive terminal applicatio...
New
Damirados
Hello everyone. After busy few months I am happy to announce v0.1.0 of Emerge &amp; Solve. They are GUI (Emerge) and State management (S...
New
netoum
Corex is an accessible, unstyled UI component library for Phoenix that integrates Zag.js state machines using Vanilla JavaScript and Live...
New
wintermeyer
There are three potential reasons for members of this forum to have a look at https://vutuv.de You are tired or annoyed of LinkedIn. Yo...
New
webofbits
Aludel - LLM Evaluation Workbench Aludel is an embeddable Phoenix LiveView dashboard for evaluating and comparing LLM prompts across mult...
New

We're in Beta

About us Mission Statement

Options

Thread Display Mode




Thread Preview

Skip Thread Previews