fix(Credo): Reduce function nesting (#2083)

* fix(Credo): Reduce function nesting

* Addressed feedback

* Changed to private def

* Added missing defp

lib/dotcom_web/controllers/schedule/line.ex

@@ -24,6 +24,19 @@ defmodule DotcomWeb.ScheduleController.Line do
     Util.log_duration(__MODULE__, :do_call, [conn, opts])
   end
 
+  defp new_or_existing_direction_id(1, _), do: 1
+  defp new_or_existing_direction_id(0, _), do: 0
+  defp new_or_existing_direction_id(_, direction_id), do: direction_id
+
+  defp parse_direction_id(%{direction_id: schedule_direction_id}, direction_id) do
+    case Integer.parse(schedule_direction_id) do
+      :error -> direction_id
+      parsed -> new_or_existing_direction_id(elem(parsed, 0), direction_id)
+    end
+  end
+
+  defp parse_direction_id(_, direction_id), do: direction_id
+
   def do_call(
         %Conn{
           assigns: %{
@@ -44,16 +57,7 @@ defmodule DotcomWeb.ScheduleController.Line do
       # URL parameters have the correct format
       schedule_direction = Map.get(conn.query_params, "schedule_direction", %{})
 
-      direction_id_value =
-        if schedule_direction["direction_id"] do
-          parsed = Integer.parse(schedule_direction["direction_id"])
-
-          if parsed !== :error and (elem(parsed, 0) === 1 or elem(parsed, 0) === 0),
-            do: elem(parsed, 0),
-            else: direction_id
-        else
-          direction_id
-        end
+      direction_id_value = parse_direction_id(schedule_direction, direction_id)
 
       update_conn(conn, route, direction_id_value)
     else

lib/dotcom_web/controllers/schedule_controller.ex

@@ -85,6 +85,21 @@ defmodule DotcomWeb.ScheduleController do
     redirect(conn, external: "https://capeflyer.com")
   end
 
+  defp format_time(nil), do: "nil"
+
+  defp format_time(time) do
+    case Timex.format(time, "{h24}:{m}") do
+      {:ok, time} -> time
+      {:error, err} -> err
+    end
+  end
+
+  defp trip_and_time_tuple(%Schedule{time: time, trip: trip}) do
+    trip_id = if is_nil(trip), do: "nil", else: Map.get(trip, :id)
+    time = format_time(time)
+    {trip_id, time}
+  end
+
   defp future_departures(schedules, %{"future_departures" => "true"} = params) do
     now = Util.now()
     # Only list schedules with time in the future. The "time" property might be
@@ -97,32 +112,17 @@ defmodule DotcomWeb.ScheduleController do
     if in_schedules == [] and length(schedules) > 0 do
       # Why were so many schedules filtered out? Probably because they're in the
       # past. But let's log the last five, to uncover other possible issues.
-      time_fn = fn t ->
-        case Timex.format(t, "{h24}:{m}") do
-          {:ok, t} -> t
-          {:error, err} -> err
-        end
-      end
 
       log_entries =
-        Enum.map(out_schedules, fn
-          %Schedule{time: t, trip: trip} ->
-            trip_id = if is_nil(trip), do: "nil", else: Map.get(trip, :id)
-
-            time =
-              if is_nil(t) do
-                "nil"
-              else
-                time_fn.(t)
-              end
-
-            {trip_id, time}
-        end)
+        Enum.map(
+          out_schedules,
+          &trip_and_time_tuple/1
+        )
         |> Enum.take(-5)
 
       _ =
         Logger.info(
-          "module=#{__MODULE__} fun=future_departures stop=#{params["stop_id"]} removed=#{inspect(log_entries)} time=#{time_fn.(now)}"
+          "module=#{__MODULE__} fun=future_departures stop=#{params["stop_id"]} removed=#{inspect(log_entries)} time=#{format_time(now)}"
         )
     end
 

lib/dotcom_web/controllers/trip_plan_controller.ex

@@ -573,6 +573,17 @@ defmodule DotcomWeb.TripPlanController do
     end)
   end
 
+  defp chunk_subway_legs({leg, _idx}) do
+    highest_fare =
+      Fares.get_fare_by_type(leg, :highest_one_way_fare)
+
+    if is_nil(highest_fare) do
+      false
+    else
+      Transfer.subway?(highest_fare.mode)
+    end
+  end
+
   @spec readjust_itinerary_with_free_fares(Itinerary.t()) :: Itinerary.t()
   def readjust_itinerary_with_free_fares(itinerary) do
     transit_legs =
@@ -589,17 +600,7 @@ defmodule DotcomWeb.TripPlanController do
       else
         Enum.chunk_by(
           legs_after_airport,
-          fn {leg, _idx} ->
-            highest_fare =
-              leg
-              |> Fares.get_fare_by_type(:highest_one_way_fare)
-
-            if is_nil(highest_fare) do
-              false
-            else
-              Transfer.subway?(highest_fare.mode)
-            end
-          end
+          &chunk_subway_legs/1
         )
         |> Enum.at(0)
       end

lib/dotcom_web/views/page_content_view.ex

@@ -68,15 +68,17 @@ defmodule DotcomWeb.CMS.PageView do
     Enum.any?(paragraphs, &right_rail_check(&1))
   end
 
+  defp teasers?(%{teasers: teasers}) do
+    if Enum.empty?(teasers), do: false, else: true
+  end
+
+  defp teasers?(_), do: true
+
   # Checks if any paragraphs have been assigned to the right rail.
   # If the paragraph is a ContentList.t(), ensure it has teasers.
   defp right_rail_check(paragraph) do
     if Paragraph.right_rail?(paragraph) do
-      if Map.has_key?(paragraph, :teasers) do
-        if Enum.empty?(paragraph.teasers), do: false, else: true
-      else
-        true
-      end
+      teasers?(paragraph)
     else
       false
     end

lib/mix/tasks/header_footer.ex

@@ -60,6 +60,18 @@ if Mix.env() in [:dev, :test] do
       make_zip()
     end
 
+    defp click_lang(_, "en"), do: nil
+
+    defp click_lang(dropdown, lang_code) do
+      dropdown
+      |> click(:middle)
+      |> find(Query.css("option[data-lang='#{lang_code}']"), fn option ->
+        Element.click(option)
+        # Translations take so long...
+        :timer.sleep(90_000)
+      end)
+    end
+
     defp make_markup(lang_code) do
       session = new_session()
 
@@ -68,17 +80,7 @@ if Mix.env() in [:dev, :test] do
         |> visit("https://dev.mbtace.com/menu")
         |> find(
           Query.css("header .custom-language-selector"),
-          fn dropdown ->
-            if lang_code != "en" do
-              dropdown
-              |> click(:middle)
-              |> find(Query.css("option[data-lang='#{lang_code}']"), fn option ->
-                Element.click(option)
-                # Translations take so long...
-                :timer.sleep(90_000)
-              end)
-            end
-          end
+          &click_lang(&1, lang_code)
         )
         |> page_source()
 
@@ -133,6 +135,25 @@ if Mix.env() in [:dev, :test] do
       {:ok, _files} = File.rm_rf("export")
     end
 
+    defp strip_base_path(nil, filename_path), do: filename_path
+
+    defp strip_base_path(base_path, filename_path) do
+      String.replace_leading(filename_path, base_path, "")
+    end
+
+    defp append_file_and_filename(filename_path, base_path, acc) do
+      filename = strip_base_path(base_path, filename_path)
+      [{String.to_charlist(filename), File.read!(filename_path)} | acc]
+    end
+
+    defp files_list_reducer(filename, path, base_path, acc) do
+      filename_path = Path.join(path, filename)
+
+      if File.dir?(filename_path),
+        do: acc ++ create_files_list(File.ls!(filename_path), filename_path, base_path),
+        else: append_file_and_filename(filename_path, base_path, acc)
+    end
+
     defp create_files_list(path) do
       # thanks https://stackoverflow.com/a/44734142
       create_files_list(File.ls!(path), path)
@@ -143,20 +164,7 @@ if Mix.env() in [:dev, :test] do
     end
 
     defp create_files_list(paths, path, base_path) do
-      Enum.reduce(paths, [], fn filename, acc ->
-        filename_path = Path.join(path, filename)
-
-        if File.dir?(filename_path) do
-          acc ++ create_files_list(File.ls!(filename_path), filename_path, base_path)
-        else
-          filenm =
-            if base_path,
-              do: String.replace_leading(filename_path, base_path, ""),
-              else: filename_path
-
-          [{String.to_charlist(filenm), File.read!(filename_path)} | acc]
-        end
-      end)
+      Enum.reduce(paths, [], &files_list_reducer(&1, path, base_path, &2))
     end
 
     defp write_mbta_file({header_or_footer, lang_code, markup}) do

lib/stops/nearby.ex

@@ -161,6 +161,21 @@ defmodule Stops.Nearby do
     {[first], rest}
   end
 
+  defp reduce_keys({:ok, {item, keys}}, {existing, all_keys}, max_count) do
+    still_valid_keys = Enum.reject(keys, &(Map.get(all_keys, &1) == max_count))
+
+    if still_valid_keys == [] do
+      {existing, all_keys}
+    else
+      updated_keys =
+        Enum.reduce(still_valid_keys, all_keys, fn key, keys ->
+          Map.update(keys, key, 1, &(&1 + 1))
+        end)
+
+      {[item | existing], updated_keys}
+    end
+  end
+
   @doc """
   Filters an enumerable such that the keys (returned by `keys_fn`) do not
   appear more than `max_count` times.
@@ -179,21 +194,7 @@ defmodule Stops.Nearby do
     {items, _} =
       enum
       |> Task.async_stream(fn item -> {item, keys_fn.(item)} end)
-      |> Enum.reduce({[], %{}}, fn {:ok, {item, keys}}, {existing, all_keys} ->
-        still_valid_keys = Enum.reject(keys, &(Map.get(all_keys, &1) == max_count))
-
-        if still_valid_keys == [] do
-          {existing, all_keys}
-        else
-          updated_keys =
-            still_valid_keys
-            |> Enum.reduce(all_keys, fn key, keys ->
-              Map.update(keys, key, 1, &(&1 + 1))
-            end)
-
-          {[item | existing], updated_keys}
-        end
-      end)
+      |> Enum.reduce({[], %{}}, &reduce_keys(&1, &2, max_count))
 
     Enum.reverse(items)
   end