Late to the party. Brute forced like others

A tiny bit of code review on the above - nothing major, mostly “here’s a shorter way to write the same ideas” tips.

• many Enum functions have a variant that lets you transform the input before doing their thing. For instance, Enum.count/2 or Enum.max_by/4. There’s a minor performance benefit of using them since the intermediate list doesn’t need to be constructed, but IMO the readability gain is better.

• Enum.map + List.flatten == Enum.flat_map, only again the intermediate list doesn’t need to be constructed.

• Most code that uses Enum.reduce with an initial value of %{} will be clearer with Map.new. I say “most” because sometimes there’s code in the block passed to reduce that returns acc unchanged, which you can’t do with Map.new

• most of the time when you want one-line-at-a-time, File.stream! will save you some typing. By default, it already splits lines. There is also a theoretical memory-usage advantage since using Stream means you don’t need every line in memory at once, but it’s unlikely to be important.

• functions are basically free: make more of them. In my experience, if you’d use a phrase to name a piece of code when discussing it with a colleague, it should probably be a separate function. For instance, here’s the “read the file in” part from my day 8 solution:

  def read(filename) do
    File.stream!(filename)
    |> Stream.map(&String.trim/1)
    |> Stream.with_index()
    |> Stream.flat_map(&parse_line/1)
    |> Map.new()
  end

  defp parse_line({line, row_index}) do
    line
    |> String.codepoints()
    |> Enum.map(&String.to_integer/1)
    |> Enum.with_index()
    |> Enum.map(fn {h, col_index} -> {{row_index, col_index}, h} end)
  end

If you wanted %Tree{} structs like in your version, you’d change that very last statement of parse_line to build one out of row_index / col_index / h values.

Another guideline I find useful: repeat yourself, find the common parts, and then make THAT a function. For instance, you might notice this pattern (placeholders in SHOUTING_CASE):

trees_DIR = GET_TREES

visibility_score_DIR =
  trees_DIR
  |> visibility_score(current)

visible_DIR? =
  trees_DIR
  |> Enum.filter(fn x -> current <= x end)
  |> Enum.empty?()

This becomes a function:

defp visibility_of(trees, current) do
  score = visibility_score(trees, current)

  flag =
    trees
    |> Enum.filter(fn x -> current <= x end)
    |> Enum.empty? # NOTE: consider using any? instead of filter + empty?

  {score, flag}
end

then the big branch of the case shortens to:

            {row, column} ->
              %{height: current, visible: _v} = Map.get(trees, {i, j})

              {visibility_score_left, visible_left?} =
                trees
                |> traverse_x(column - 1, 0, row)
                |> visibility_of(current)

              {visibility_score_right, visible_right?} =
                trees
                |> traverse_x(column + 1, col_count, row)
                |> visibility_of(current)

              {visibility_score_up, visible_up?} =
                trees
                |> traverse_y(row - 1, 0, column)
                |> visibility_of(current)

              {visibility_score_down, visible_down?} =
                trees
                |> traverse_y(row + 1, row_count, column)
                |> visibility_of(current)

              %{
                {i, j} => %Tree{
                  height: current,
                  visible: visible_up? || visible_down? || visible_left? || visible_right?,
                  score:
                    visibility_score_down * visibility_score_left * visibility_score_right *
                      visibility_score_up
                }
              }

Writing things this way makes it clearer that only the trees change between the four copies of the code.

Something I keep having use for in these problems where you have to walk around in a matrix is to use a list of “vectors” instead of hardcoding the movements.
This is for part 2, made a more “clever”/convoluted solution for part 1… but I had no use for in part 2.

defmodule VisibilityChecker do
  def max_visibility(grid) do
    heights = for {rows, y} <- Enum.with_index(grid), 
      {h, x} <- Enum.with_index(rows), into: %{} do
      {{x, y}, h}
    end

    heights
    |> Stream.map(&elem(&1, 0))
    |> Stream.map(&score(&1, heights))
    |> Enum.max()
  end

  @directions [{-1, 0},{1, 0},{0, -1},{0, 1},]
  defp score(pos, heights) do
    for dir <- @directions, reduce: 1 do
      score -> score * visible(heights, pos, dir, heights[pos], 0)
    end
  end

  defp visible(heights, pos, direction, max_height, line_height) do
    new_pos = translate(pos, direction)
    case heights[new_pos] do
      nil -> 0
      tree_height when tree_height >= max_height -> 1
      tree_height when tree_height >= line_height -> 
        1 + visible(heights, new_pos, direction, max_height, tree_height)
      _ -> 
        1 + visible(heights, new_pos, direction, max_height, line_height)
    end
  end

  defp translate({x, y}, {dx, dy}), do: {x + dx, y + dy}
end

# input is a list of lists with the three heights [ [ 1, 2], [ 3, 4 ]]
#  
# 1 2
# 3 4
#
# would be 
# [
#.  [ 1, 2 ],
#   [ 3, 4 ]
# ]
VisibilityChecker.max_visibility(input)

Done with Livebook