Refactoring, formatting, doctests

lib/bowlarama.ex

@@ -13,52 +13,41 @@ defmodule Bowlarama do
       iex(8)> Bowlarama.score_sheet('./fixtures/scores.txt')
   """
   def score_sheet(file) do
-    case File.read(file) do
-      {:ok, binary} -> process(binary)
+    with {:ok, binary} <- File.read(file) do
+      players_with_scores =
+        binary
+        |> String.split("\n", trim: true)
+        # Create a list of lists: [["Jeff", "10"], ["John", "3"], [], ...]
+        |> Enum.map(&String.split(&1, " "))
+
+      players_with_scores
+      # Extract players names into a list: ["Jeff", "John"]
+      |> extract_players()
+      |> Players.assign_scores_to(players_with_scores)
+      |> ScoreCard.print()
+    else
       {:error, _reason} -> "File does not exist"
     end
   end
 
-  defp process(lines) do
-    list_of_players = String.split(lines, "\n", trim: true)
-
-    # Create a list of lists: [["Jeff", "10"], ["John", "3"], [], ...]
-    players_with_scores = Enum.map(list_of_players, fn(score) -> String.split(score, " ") end)
-
-    # Extract players names into a list: ["Jeff", "John"]
-    players = extract_players(players_with_scores)
-
-    Players.assign_scores_to(players, players_with_scores) |>
-    ScoreCard.print()
-  end
-
   defp extract_players(nested_lists) do
     nested_lists
     |> List.flatten()
     |> Enum.uniq()
-    |> Enum.filter(fn(ele) -> extract(ele) end)
-  end
-
-  defp extract(item) do
-    case player_from_string(item) do
-      {:error, name} -> name
-      {:ok, _integer} -> false
-      _ -> false
-    end
+    |> Enum.filter(&player_from_string/1)
   end
 
   # Receives a list like this:
   # ["Jeff", "10", "John", "8", "F"]
   #
   # If the string can be converted to integer, it isn't a name.
   # Too hacky for me. Works good enough, though.
-  defp player_from_string("F"), do: {:ok, 0}
+  defp player_from_string("F"), do: false
 
   defp player_from_string(string) do
-    try do
-      {:ok, String.to_integer(string)}
-    rescue
-      _e in ArgumentError -> {:error, string}
+    case Integer.parse(string) do
+      :error -> string
+      _ -> false
     end
   end
 end

lib/frame.ex

@@ -11,7 +11,7 @@ defmodule Frame do
 
   Considers pairing for strikes by placing a number 0 besides the 10
   to simplify score calculation in Score module.
-
+  Example made that way will work as doctest.
   ## Example
 
       iex> Frame.frames(["10","7","3","9","0","10","0","8","8","2","0","6","10","10","10"])
@@ -32,8 +32,4 @@ defmodule Frame do
   defp pair_up([first | rest], new_list) do
     pair_up(rest, [first | new_list])
   end
-
-  defp pair_up([], new_list) do
-    Enum.reverse(new_list)
-  end
 end

lib/invalid_roll_error.ex

@@ -1,3 +1,3 @@
 defmodule InvalidRollError do
-  defexception message: "Invalid roll anotation"
+  defexception message: "Invalid roll annotation"
 end

lib/players.ex

@@ -24,25 +24,26 @@ defmodule Players do
     player_2 = List.last(players)
 
     {
-      %{ player_1 => assign(player_1, scores) },
-      %{ player_2 => assign(player_2, scores) }
+      %{player_1 => assign(player_1, scores)},
+      %{player_2 => assign(player_2, scores)}
     }
   end
 
   defp assign(player, scores) do
     scores
-    |> Enum.reduce([], fn(pair, acc) -> [filter_scores(player, pair) | acc] end)
+    |> Enum.reduce([], fn pair, acc -> [filter_scores(player, pair) | acc] end)
     |> Frame.frames()
     |> organize_scores()
   end
 
   defp filter_scores(player, [name | _rest]) when name != player, do: nil
 
   # Enum.reduce() va a devolverlos en orden invertido
-  defp filter_scores(player, [name, score | _]) when name == player, do: score
+  defp filter_scores(_, [_, score | _]), do: score
 
   defp organize_scores(scores) do
-    Enum.reverse(scores)
-    |> Enum.reject(fn(ele) -> is_nil(ele) end)
+    scores
+    |> Enum.reverse()
+    |> Enum.reject(&is_nil/1)
   end
 end

lib/score.ex

@@ -11,19 +11,16 @@ defmodule Score do
       iex> Score.convert_to_numbers(["10", "2", "3", "5"])
       [10, 2, 3, 5]
   """
-  def convert_to_numbers(rolls) do
-    Enum.map(rolls, fn(roll) -> validate(roll) end)
-  end
+  def convert_to_numbers(rolls), do: rolls |> Enum.map(&validate/1)
 
   @doc """
-  Calcualtes scores for a player's pinfalls.
+  Calculates scores for a player's pinfalls.
 
   It considers strikes and spares rules.
 
   ## Examples
 
   For a list of scores:
-
       iex> Score.calculate([[10,0], [7,3], [9,0], [10,0], [0,8], [8,2], [0,6], [10,0], [10,0], [0,10,8]])
       [20, 39, 48, 66, 74, 84, 90, 110, 130, 140]
   """
@@ -43,8 +40,8 @@ defmodule Score do
   end
 
   # For Strikes and empty result list
-  defp calculate([[fst, sec | _], [n1, n2 | _] | rest], []) when fst == 10 do
-    calculate([[n1, n2] | rest], [fst + sec + n1 + n2])
+  defp calculate([[10, sec | _], [n1, n2 | _] | rest], []) do
+    calculate([[n1, n2] | rest], [10 + sec + n1 + n2])
   end
 
   # For Spares and empty result list
@@ -57,8 +54,8 @@ defmodule Score do
   end
 
   # For Strikes
-  defp calculate([[fst, sec | _], [n1, n2 | _] | rest], [ff | tail]) when fst == 10 do
-    next_score = ff + fst + sec + n1 + n2
+  defp calculate([[10, sec | _], [n1, n2 | _] | rest], [ff | tail]) do
+    next_score = ff + 10 + sec + n1 + n2
 
     calculate([[n1, n2] | rest], [next_score, ff | tail])
   end

lib/score_card.ex

@@ -7,47 +7,43 @@ defmodule ScoreCard do
   @double_tab "\t\t"
 
   def print({p1, p2} = _players_and_scores) do
-    IO.puts("Frame#{@double_tab}" <> match_frames())
+    IO.puts("Frame#{@double_tab}" <> Enum.join(1..10, @double_tab))
 
-    Enum.each(p1, fn({player, scores}) -> results(player, scores) end)
-    Enum.each(p2, fn({player, scores}) -> results(player, scores) end)
+    Enum.each(p1, fn {player, scores} -> results(player, scores) end)
+    Enum.each(p2, fn {player, scores} -> results(player, scores) end)
   end
 
-  defp match_frames, do: Enum.join([1,2,3,4,5,6,7,8,9,10], @double_tab)
-
   defp results(player, rolls) do
     IO.puts(player)
     IO.puts("Pinfalls#{@single_tab}" <> print_rolls(rolls))
     IO.puts("Score#{@double_tab}" <> print_scores(rolls))
   end
 
   defp print_rolls(rolls) do
-    Enum.chunk_every(rolls, 2, 2, :discard) |>
-    Enum.map(fn(frame_rolls) -> strike_or_spare(frame_rolls) end) |>
-    Enum.join(@single_tab)
+    rolls
+    |> Enum.chunk_every(2, 2, :discard)
+    |> Enum.map(&strike_or_spare/1)
+    |> Enum.join(@single_tab)
   end
 
   defp strike_or_spare(["10" | _last]), do: @single_tab <> "X"
-
-  defp strike_or_spare([first , last | _rest]) when first == "F" or last == "F" do
-    "#{first}#{@single_tab}#{last}"
-  end
+  defp strike_or_spare(["F", last | _rest]), do: "F#{@single_tab}#{last}"
+  defp strike_or_spare([first, "F" | _rest]), do: "#{first}#{@single_tab}F"
 
   # No retorno @single_tab al final porque ya en print_rolls lo hago en la última línea
-  defp strike_or_spare([first , last | _rest]) do
+  defp strike_or_spare([first, last | _rest]) do
     result = String.to_integer(first) + String.to_integer(last)
 
-    if result == 10 do
-      "#{first}" <> @single_tab <> "/"
-    else
-      "#{first}#{@single_tab}#{last}"
-    end
+    last = if result == 10, do: "/", else: last
+    "#{first}#{@single_tab}#{last}"
   end
 
   defp print_scores(rolls) do
-    Score.convert_to_numbers(rolls) |> # Aquí uso los _rolls_ puros y controlo letra F
-    Enum.chunk_every(2, 2, :discard) |>
-    Score.calculate() |>
-    Enum.join(@double_tab)
+    # Aquí uso los _rolls_ puros y controlo letra F
+    rolls
+    |> Score.convert_to_numbers()
+    |> Enum.chunk_every(2, 2, :discard)
+    |> Score.calculate()
+    |> Enum.join(@double_tab)
   end
 end

test/test_helper.exs

@@ -1 +1,2 @@
+ExUnit.configure(exclude: [pending: true])
 ExUnit.start()