Fix credo warnings and start on design suggestions

Warnings fixed (13 issues):
- Replace expensive length/1 checks with empty list comparisons
- Remove redundant n == n comparison in is_finite guards
- Use proper type notation in @spec instead of struct literal

Design suggestions (3/13 files):
- Add aliases for nested modules in historical_loader, page_controller, mobile_channel

Remaining: 10 design suggestions, 42 refactoring opportunities (complex functions)
This commit is contained in:
Graham McIntire 2026-02-09 11:23:01 -06:00
parent 81a4b7b815
commit dc0bc7bbdf
No known key found for this signature in database
11 changed files with 29 additions and 23 deletions

View file

@ -71,16 +71,16 @@ defmodule Aprsme.Cluster.LeaderElection do
else else
connected_nodes = Node.list() connected_nodes = Node.list()
if length(connected_nodes) > 0 do if connected_nodes == [] do
Logger.info("Cluster formed with #{length(connected_nodes)} other nodes: #{inspect(connected_nodes)}")
Logger.info("Proceeding with leader election")
Process.send_after(self(), :attempt_election, 100)
{:noreply, %{state | election_forced: true}}
else
Logger.debug("Cluster not yet formed - waiting...") Logger.debug("Cluster not yet formed - waiting...")
# Check again in 2 seconds # Check again in 2 seconds
Process.send_after(self(), :check_cluster_and_elect, 2_000) Process.send_after(self(), :check_cluster_and_elect, 2_000)
{:noreply, state} {:noreply, state}
else
Logger.info("Cluster formed with #{length(connected_nodes)} other nodes: #{inspect(connected_nodes)}")
Logger.info("Proceeding with leader election")
Process.send_after(self(), :attempt_election, 100)
{:noreply, %{state | election_forced: true}}
end end
end end
end end
@ -91,7 +91,7 @@ defmodule Aprsme.Cluster.LeaderElection do
if not state.election_forced and not state.is_leader do if not state.election_forced and not state.is_leader do
connected_nodes = Node.list() connected_nodes = Node.list()
if length(connected_nodes) == 0 do if connected_nodes == [] do
Logger.warning( Logger.warning(
"Cluster formation timeout reached after #{@max_cluster_wait}ms with no connected nodes. " <> "Cluster formation timeout reached after #{@max_cluster_wait}ms with no connected nodes. " <>
"Proceeding with leader election in single-node mode to ensure APRS-IS connection." "Proceeding with leader election in single-node mode to ensure APRS-IS connection."

View file

@ -308,7 +308,7 @@ defmodule Aprsme.PacketConsumer do
Logger.error("Batch insert failed: #{inspect(error)}") Logger.error("Batch insert failed: #{inspect(error)}")
# Log sample packet to help debug field issues # Log sample packet to help debug field issues
if length(valid_packets) > 0 do if valid_packets != [] do
sample_packet = List.first(valid_packets) sample_packet = List.first(valid_packets)
Logger.error("Sample packet fields: #{inspect(Map.keys(sample_packet))}") Logger.error("Sample packet fields: #{inspect(Map.keys(sample_packet))}")
Logger.error("Sample packet data: #{inspect(sample_packet)}") Logger.error("Sample packet data: #{inspect(sample_packet)}")

View file

@ -45,7 +45,7 @@ defmodule Aprsme.PacketProducer do
end end
end end
defp dispatch_events(buffer, demand) when demand > 0 and length(buffer) > 0 do defp dispatch_events(buffer, demand) when demand > 0 and buffer != [] do
{events, remaining} = Enum.split(buffer, demand) {events, remaining} = Enum.split(buffer, demand)
{events, remaining, demand - length(events)} {events, remaining, demand - length(events)}
end end

View file

@ -52,6 +52,8 @@ defmodule AprsmeWeb.MobileChannel do
""" """
use AprsmeWeb, :channel use AprsmeWeb, :channel
alias AprsmeWeb.MapLive.MapHelpers
require Logger require Logger
@impl true @impl true
@ -277,7 +279,7 @@ defmodule AprsmeWeb.MobileChannel do
defp build_mobile_packet(packet) do defp build_mobile_packet(packet) do
# Extract coordinates # Extract coordinates
{lat, lon, _data_extended} = AprsmeWeb.MapLive.MapHelpers.get_coordinates(packet) {lat, lon, _data_extended} = MapHelpers.get_coordinates(packet)
# Build minimal packet data for mobile # Build minimal packet data for mobile
%{ %{
@ -465,7 +467,7 @@ defmodule AprsmeWeb.MobileChannel do
Logger.info("Loaded #{length(packets)} historical packets for callsign #{callsign}") Logger.info("Loaded #{length(packets)} historical packets for callsign #{callsign}")
# Send historical packets to client # Send historical packets to client
if length(packets) > 0 do if packets != [] do
Enum.each(packets, fn packet -> Enum.each(packets, fn packet ->
packet_data = build_mobile_packet(packet) packet_data = build_mobile_packet(packet)
push(socket, "packet", packet_data) push(socket, "packet", packet_data)

View file

@ -2,6 +2,8 @@ defmodule AprsmeWeb.PageController do
@moduledoc false @moduledoc false
use AprsmeWeb, :controller use AprsmeWeb, :controller
alias Aprsme.Cluster.LeaderElection
def home(conn, _params) do def home(conn, _params) do
render(conn, :home) render(conn, :home)
end end
@ -80,7 +82,7 @@ defmodule AprsmeWeb.PageController do
def status_json(conn, _params) do def status_json(conn, _params) do
# Get cluster-wide APRS-IS connection status # Get cluster-wide APRS-IS connection status
aprs_status = Aprsme.Cluster.LeaderElection.get_cluster_aprs_status() aprs_status = LeaderElection.get_cluster_aprs_status()
# Get application version # Get application version
version = :aprsme |> Application.spec(:vsn) |> List.to_string() version = :aprsme |> Application.spec(:vsn) |> List.to_string()

View file

@ -139,7 +139,7 @@ defmodule AprsmeWeb.MapLive.DataBuilder do
defp valid_coordinates?(_, _), do: false defp valid_coordinates?(_, _), do: false
defp is_finite(n) when is_float(n), do: n != :infinity and n != :neg_infinity and n == n defp is_finite(n) when is_float(n), do: n != :infinity and n != :neg_infinity
defp is_finite(n) when is_integer(n), do: true defp is_finite(n) when is_integer(n), do: true
@doc """ @doc """

View file

@ -6,8 +6,10 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do
import Phoenix.Component, only: [assign: 3] import Phoenix.Component, only: [assign: 3]
alias Aprsme.Packets alias Aprsme.Packets
alias AprsmeWeb.Live.Shared.CoordinateUtils
alias AprsmeWeb.Live.Shared.PacketUtils, as: SharedPacketUtils alias AprsmeWeb.Live.Shared.PacketUtils, as: SharedPacketUtils
alias AprsmeWeb.MapLive.DataBuilder alias AprsmeWeb.MapLive.DataBuilder
alias AprsmeWeb.MapLive.RfPath
alias Phoenix.LiveView alias Phoenix.LiveView
alias Phoenix.LiveView.Socket alias Phoenix.LiveView.Socket
@ -219,7 +221,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do
# Filter out packets with invalid coordinates before processing # Filter out packets with invalid coordinates before processing
valid_packets = valid_packets =
Enum.filter(historical_packets, fn packet -> Enum.filter(historical_packets, fn packet ->
{lat, lon, _} = AprsmeWeb.Live.Shared.CoordinateUtils.get_coordinates(packet) {lat, lon, _} = CoordinateUtils.get_coordinates(packet)
is_number(lat) and is_number(lon) and is_number(lat) and is_number(lon) and
lat >= -90 and lat <= 90 and lon >= -180 and lon <= 180 and lat >= -90 and lat <= 90 and lon >= -180 and lon <= 180 and
@ -416,7 +418,7 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do
packets packets
|> Enum.flat_map(fn packet -> |> Enum.flat_map(fn packet ->
path = Map.get(packet, :path, "") path = Map.get(packet, :path, "")
AprsmeWeb.MapLive.RfPath.parse_rf_path(path) RfPath.parse_rf_path(path)
end) end)
|> Enum.uniq() |> Enum.uniq()
|> Enum.reject(&(&1 == "")) |> Enum.reject(&(&1 == ""))
@ -431,6 +433,6 @@ defmodule AprsmeWeb.MapLive.HistoricalLoader do
socket socket
end end
defp is_finite(n) when is_float(n), do: n != :infinity and n != :neg_infinity and n == n defp is_finite(n) when is_float(n), do: n != :infinity and n != :neg_infinity
defp is_finite(n) when is_integer(n), do: true defp is_finite(n) when is_integer(n), do: true
end end

View file

@ -428,14 +428,14 @@ defmodule AprsmeWeb.MapLive.Index do
# Send event to draw the RF path lines # Send event to draw the RF path lines
socket = socket =
if length(path_station_positions) > 0 do if path_station_positions == [] do
socket
else
push_event(socket, "draw_rf_path", %{ push_event(socket, "draw_rf_path", %{
station_lat: lat_float, station_lat: lat_float,
station_lng: lng_float, station_lng: lng_float,
path_stations: path_station_positions path_stations: path_station_positions
}) })
else
socket
end end
{:noreply, socket} {:noreply, socket}

View file

@ -28,7 +28,7 @@ defmodule AprsmeWeb.UserAuth do
disconnected on log out. The line can be safely removed disconnected on log out. The line can be safely removed
if you are not using LiveView. if you are not using LiveView.
""" """
@spec log_in_user(Plug.Conn.t(), %Aprsme.Accounts.User{}, map()) :: Plug.Conn.t() @spec log_in_user(Plug.Conn.t(), Aprsme.Accounts.User.t(), map()) :: Plug.Conn.t()
def log_in_user(conn, user, params \\ %{}) do def log_in_user(conn, user, params \\ %{}) do
token = Accounts.generate_user_session_token(user) token = Accounts.generate_user_session_token(user)
user_return_to = get_session(conn, :user_return_to) user_return_to = get_session(conn, :user_return_to)

View file

@ -24,7 +24,7 @@ defmodule Aprsme.Packets.ClusteringTest do
result = Clustering.cluster_packets(packets, 7, %{}) result = Clustering.cluster_packets(packets, 7, %{})
assert {:heat_map, clusters} = result assert {:heat_map, clusters} = result
assert is_list(clusters) assert is_list(clusters)
assert length(clusters) > 0 assert clusters != []
end end
test "returns clustered data when zoom level is 8 or less" do test "returns clustered data when zoom level is 8 or less" do
@ -37,7 +37,7 @@ defmodule Aprsme.Packets.ClusteringTest do
result = Clustering.cluster_packets(packets, 7, %{}) result = Clustering.cluster_packets(packets, 7, %{})
assert {:heat_map, clusters} = result assert {:heat_map, clusters} = result
assert is_list(clusters) assert is_list(clusters)
assert length(clusters) > 0 assert clusters != []
# Check cluster structure # Check cluster structure
[first_cluster | _] = clusters [first_cluster | _] = clusters

View file

@ -339,7 +339,7 @@ defmodule Aprsme.PacketsTest do
test "returns stations ordered by distance", %{stations: _stations} do test "returns stations ordered by distance", %{stations: _stations} do
results = Packets.get_nearby_stations(39.0, -98.0) results = Packets.get_nearby_stations(39.0, -98.0)
assert length(results) > 0 assert results != []
# Results should be ordered by distance # Results should be ordered by distance
[first | _] = results [first | _] = results
assert first.sender in ["NEAR1", "NEAR2"] assert first.sender in ["NEAR1", "NEAR2"]