Refactor conditionals to use multi-clause pattern matching

Replace case/cond/if blocks with multi-clause function dispatch
and pattern matching in function heads across 9 modules:
- WeatherUnits: case to multi-clause unit_system/1, unit_labels via do_unit_labels/1
- PacketUtils: if to pattern match on %{"id" => id}, multi-clause threshold/callsign
- SetLocale: case to multi-clause extract_locale/1
- IPGeolocation: if/cond to pattern match on conn, binary prefix matching for private_ip?
- DeviceIdentification: cond to extracted pattern_matches?/2
- MobileChannel: cond to multi-clause do_callsign_match/2
- PacketProcessor: cond to dispatch_visibility/6 on {in_bounds, has_marker} tuple
- PacketManager: destructure packet_state in heads, multi-clause maybe_cleanup/3
- AprsSymbol: case/if to multi-clause get_table_id/1, normalize_symbol_code/1
This commit is contained in:
Graham McIntire 2026-02-10 17:15:49 -06:00
parent 6b624e1365
commit 4366fa36da
No known key found for this signature in database
9 changed files with 171 additions and 256 deletions

View file

@ -221,28 +221,23 @@ defmodule Aprsme.DeviceIdentification do
end
Enum.find(devices, fn device ->
pattern = device.identifier
cond do
String.contains?(pattern, "?") ->
try do
regex = wildcard_pattern_to_regex(pattern)
Regex.match?(regex, identifier)
rescue
_e in Regex.CompileError ->
false
end
String.contains?(pattern, "*") ->
# Compare literally if pattern contains * but not ?
pattern == identifier
true ->
pattern == identifier
end
pattern_matches?(device.identifier, identifier)
end)
end
defp pattern_matches?(pattern, identifier) do
if String.contains?(pattern, "?") do
try do
regex = wildcard_pattern_to_regex(pattern)
Regex.match?(regex, identifier)
rescue
_e in Regex.CompileError -> false
end
else
pattern == identifier
end
end
# Converts a pattern with ? wildcards to a regex
defp wildcard_pattern_to_regex(pattern) when is_binary(pattern) do
# Replace ? with a placeholder, escape all regex metacharacters except the placeholder,

View file

@ -31,38 +31,37 @@ defmodule AprsmeWeb.AprsSymbol do
background_size: String.t()
}
def get_sprite_info(symbol_table, symbol_code) do
# For overlay symbols (A-Z, 0-9), display the base symbol with overlay
if symbol_table && String.match?(symbol_table, ~r/^[A-Z0-9]$/) do
# Use the base symbol from the overlay table, not the overlay character itself
get_overlay_base_symbol_info(symbol_code)
else
# Normal symbol table processing
symbol_table = normalize_symbol_table(symbol_table)
symbol_code = normalize_symbol_code(symbol_code)
compute_sprite_info(overlay_symbol?(symbol_table), symbol_table, symbol_code)
end
# Map symbol table to sprite file ID
table_id = get_table_id(symbol_table)
defp overlay_symbol?(table) when is_binary(table), do: String.match?(table, ~r/^[A-Z0-9]$/)
defp overlay_symbol?(_), do: false
sprite_file = "/aprs-symbols/aprs-symbols-128-#{table_id}@2x.png"
defp compute_sprite_info(true, _symbol_table, symbol_code) do
get_overlay_base_symbol_info(symbol_code)
end
# Get symbol position using ASCII-based calculation
symbol_code_ord = get_symbol_code_ord(symbol_code)
defp compute_sprite_info(false, symbol_table, symbol_code) do
symbol_table = normalize_symbol_table(symbol_table)
symbol_code = normalize_symbol_code(symbol_code)
index = symbol_code_ord - 33
safe_index = max(0, min(index, 93))
table_id = get_table_id(symbol_table)
sprite_file = "/aprs-symbols/aprs-symbols-128-#{table_id}@2x.png"
# Calculate positioning for 16-column grid
column = rem(safe_index, 16)
row = div(safe_index, 16)
x = -column * 128
y = -row * 128
symbol_code_ord = get_symbol_code_ord(symbol_code)
index = symbol_code_ord - 33
safe_index = max(0, min(index, 93))
%{
sprite_file: sprite_file,
background_position: "#{x / 4}px #{y / 4}px",
background_size: "512px 192px"
}
end
column = rem(safe_index, 16)
row = div(safe_index, 16)
x = -column * 128
y = -row * 128
%{
sprite_file: sprite_file,
background_position: "#{x / 4}px #{y / 4}px",
background_size: "512px 192px"
}
end
@doc """
@ -188,9 +187,9 @@ defmodule AprsmeWeb.AprsSymbol do
">"
"""
@spec normalize_symbol_code(String.t() | nil) :: String.t()
def normalize_symbol_code(symbol_code) do
if symbol_code && symbol_code != "", do: symbol_code, else: ">"
end
def normalize_symbol_code(nil), do: ">"
def normalize_symbol_code(""), do: ">"
def normalize_symbol_code(symbol_code), do: symbol_code
@doc """
Maps a symbol table to its sprite file ID.
@ -207,18 +206,10 @@ defmodule AprsmeWeb.AprsSymbol do
"2"
"""
@spec get_table_id(String.t()) :: String.t()
def get_table_id(symbol_table) do
case symbol_table do
# Primary table
"/" -> "0"
# Alternate table
"\\" -> "1"
# Overlay table (A-Z, 0-9)
"]" -> "2"
# Default to primary table
_ -> "0"
end
end
def get_table_id("/"), do: "0"
def get_table_id("\\"), do: "1"
def get_table_id("]"), do: "2"
def get_table_id(_), do: "0"
@doc """
Renders an APRS symbol as HTML for use in Leaflet markers.

View file

@ -478,27 +478,19 @@ defmodule AprsmeWeb.MobileChannel do
end
defp callsign_matches?(packet_callsign, tracked_callsign) do
# Normalize both to uppercase
packet_callsign = String.upcase(packet_callsign)
tracked_callsign = String.upcase(tracked_callsign)
normalized_packet = String.upcase(packet_callsign)
normalized_tracked = String.upcase(tracked_callsign)
do_callsign_match(normalized_packet, normalized_tracked)
end
cond do
# Exact match
packet_callsign == tracked_callsign ->
true
defp do_callsign_match(callsign, callsign), do: true
# Wildcard match (e.g., "W5ISP*" matches "W5ISP-9")
String.contains?(tracked_callsign, "*") ->
pattern = String.replace(tracked_callsign, "*", "")
String.starts_with?(packet_callsign, pattern)
# Base callsign match (e.g., "W5ISP" matches "W5ISP-9")
String.starts_with?(packet_callsign, tracked_callsign <> "-") ->
true
# No match
true ->
false
defp do_callsign_match(packet_callsign, tracked_callsign) do
if String.contains?(tracked_callsign, "*") do
pattern = String.replace(tracked_callsign, "*", "")
String.starts_with?(packet_callsign, pattern)
else
String.starts_with?(packet_callsign, tracked_callsign <> "-")
end
end

View file

@ -35,55 +35,43 @@ defmodule AprsmeWeb.MapLive.PacketManager do
Add packets to visible storage with memory management.
Returns updated state and packet IDs for broadcasting.
"""
def add_visible_packets(packet_state, packets) when is_list(packets) do
# Store packets and get IDs
def add_visible_packets(%{visible_packet_ids: visible_ids, packet_count: packet_count} = packet_state, packets)
when is_list(packets) do
packet_ids = PacketStore.store_packets(packets)
new_visible_ids = packet_ids ++ visible_ids
# Update visible packet tracking
new_visible_ids = packet_ids ++ packet_state.visible_packet_ids
# Apply memory management
{managed_ids, cleanup_needed} = apply_memory_limits(new_visible_ids, @max_visible_packets)
updated_state = %{
packet_state
| visible_packet_ids: managed_ids,
packet_count: Map.put(packet_state.packet_count, :visible, length(managed_ids))
packet_count: Map.put(packet_count, :visible, length(managed_ids))
}
# Clean up if needed
final_state =
if cleanup_needed do
perform_cleanup(updated_state, :visible)
else
updated_state
end
final_state = maybe_cleanup(updated_state, cleanup_needed, :visible)
{final_state, packet_ids}
end
@doc """
Add packets to historical storage with sliding window management.
"""
def add_historical_packets(packet_state, packets) when is_list(packets) do
def add_historical_packets(
%{historical_packet_ids: historical_ids, packet_count: packet_count} = packet_state,
packets
)
when is_list(packets) do
packet_ids = PacketStore.store_packets(packets)
new_historical_ids = packet_ids ++ historical_ids
new_historical_ids = packet_ids ++ packet_state.historical_packet_ids
{managed_ids, cleanup_needed} = apply_memory_limits(new_historical_ids, @max_historical_packets)
updated_state = %{
packet_state
| historical_packet_ids: managed_ids,
packet_count: Map.put(packet_state.packet_count, :historical, length(managed_ids))
packet_count: Map.put(packet_count, :historical, length(managed_ids))
}
final_state =
if cleanup_needed do
perform_cleanup(updated_state, :historical)
else
updated_state
end
final_state = maybe_cleanup(updated_state, cleanup_needed, :historical)
{final_state, packet_ids}
end
@ -165,15 +153,15 @@ defmodule AprsmeWeb.MapLive.PacketManager do
end
end
defp perform_cleanup(packet_state, _type) do
defp maybe_cleanup(state, false, _type), do: state
defp maybe_cleanup(%{last_cleanup: last_cleanup} = state, true, _type) do
current_time = System.monotonic_time(:millisecond)
# Only cleanup if enough time has passed to avoid excessive cleanup
# 30 seconds
if current_time - packet_state.last_cleanup > 30_000 do
%{packet_state | last_cleanup: current_time}
if current_time - last_cleanup > 30_000 do
%{state | last_cleanup: current_time}
else
packet_state
state
end
end

View file

@ -51,35 +51,40 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
"""
@spec handle_packet_visibility(map(), number() | nil, number() | nil, binary(), Socket.t()) :: Socket.t()
def handle_packet_visibility(packet, lat, lon, callsign_key, socket) do
cond do
should_remove_marker?(lat, lon, callsign_key, socket) ->
remove_marker_from_map(callsign_key, socket)
visibility_action(packet, lat, lon, callsign_key, socket)
end
should_add_marker?(lat, lon, callsign_key, socket) ->
handle_valid_postgres_packet(packet, lat, lon, socket)
defp visibility_action(packet, lat, lon, callsign_key, socket) when not is_nil(lat) and not is_nil(lon) do
in_bounds = within_bounds?(%{lat: lat, lon: lon}, socket.assigns.map_bounds)
has_marker = Map.has_key?(socket.assigns.visible_packets, callsign_key)
dispatch_visibility({in_bounds, has_marker}, packet, lat, lon, callsign_key, socket)
end
should_update_marker?(lat, lon, callsign_key, socket) ->
# Marker exists and is within bounds - check if there's significant movement
existing_packet = socket.assigns.visible_packets[callsign_key]
{existing_lat, existing_lon, _} = CoordinateUtils.get_coordinates(existing_packet)
defp visibility_action(_packet, _lat, _lon, _callsign_key, socket), do: socket
# Check if we have valid existing coordinates
if is_number(existing_lat) and is_number(existing_lon) and
GeoUtils.significant_movement?(existing_lat, existing_lon, lat, lon, 50) do
# Significant movement detected (more than 50 meters), update the marker
handle_valid_postgres_packet(packet, lat, lon, socket)
else
# Just GPS drift (less than 50 meters) or invalid coordinates,
# update the packet data but don't send visual update
new_visible_packets = Map.put(socket.assigns.visible_packets, callsign_key, packet)
assign(socket, :visible_packets, new_visible_packets)
end
defp dispatch_visibility({false, true}, _packet, _lat, _lon, callsign_key, socket) do
remove_marker_from_map(callsign_key, socket)
end
true ->
socket
defp dispatch_visibility({true, false}, packet, lat, lon, _callsign_key, socket) do
handle_valid_postgres_packet(packet, lat, lon, socket)
end
defp dispatch_visibility({true, true}, packet, lat, lon, callsign_key, socket) do
existing_packet = socket.assigns.visible_packets[callsign_key]
{existing_lat, existing_lon, _} = CoordinateUtils.get_coordinates(existing_packet)
if is_number(existing_lat) and is_number(existing_lon) and
GeoUtils.significant_movement?(existing_lat, existing_lon, lat, lon, 50) do
handle_valid_postgres_packet(packet, lat, lon, socket)
else
new_visible_packets = Map.put(socket.assigns.visible_packets, callsign_key, packet)
assign(socket, :visible_packets, new_visible_packets)
end
end
defp dispatch_visibility(_, _packet, _lat, _lon, _callsign_key, socket), do: socket
@doc """
Handle valid postgres packet by adding it to visible packets and displaying it.
"""
@ -128,24 +133,6 @@ defmodule AprsmeWeb.MapLive.PacketProcessor do
# Private helper functions
defp should_remove_marker?(lat, lon, callsign_key, socket) do
!is_nil(lat) and !is_nil(lon) and
Map.has_key?(socket.assigns.visible_packets, callsign_key) and
not within_bounds?(%{lat: lat, lon: lon}, socket.assigns.map_bounds)
end
defp should_add_marker?(lat, lon, callsign_key, socket) do
!is_nil(lat) and !is_nil(lon) and
not Map.has_key?(socket.assigns.visible_packets, callsign_key) and
within_bounds?(%{lat: lat, lon: lon}, socket.assigns.map_bounds)
end
defp should_update_marker?(lat, lon, callsign_key, socket) do
!is_nil(lat) and !is_nil(lon) and
Map.has_key?(socket.assigns.visible_packets, callsign_key) and
within_bounds?(%{lat: lat, lon: lon}, socket.assigns.map_bounds)
end
# Use shared bounds utility
defp within_bounds?(coords, bounds) do
BoundsUtils.within_bounds?(coords, bounds)

View file

@ -10,11 +10,8 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do
Get unique callsign key from packet.
"""
@spec get_callsign_key(map()) :: binary()
def get_callsign_key(packet) do
if Map.has_key?(packet, "id"),
do: to_string(packet["id"]),
else: [:positive] |> System.unique_integer() |> to_string()
end
def get_callsign_key(%{"id" => id}), do: to_string(id)
def get_callsign_key(_packet), do: [:positive] |> System.unique_integer() |> to_string()
@doc """
Prune oldest packets from a map to enforce memory limits.
@ -155,18 +152,13 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do
Check if a packet is within the time threshold (not too old).
"""
@spec packet_within_time_threshold?(struct(), any()) :: boolean()
def packet_within_time_threshold?(packet, threshold) do
case packet do
%{received_at: received_at} when not is_nil(received_at) ->
threshold_dt = convert_threshold_to_datetime(threshold)
DateTime.compare(received_at, threshold_dt) in [:gt, :eq]
_ ->
# If no timestamp, treat as current
true
end
def packet_within_time_threshold?(%{received_at: received_at}, threshold) when not is_nil(received_at) do
threshold_dt = convert_threshold_to_datetime(threshold)
DateTime.compare(received_at, threshold_dt) in [:gt, :eq]
end
def packet_within_time_threshold?(_packet, _threshold), do: true
defp convert_threshold_to_datetime(threshold) when is_integer(threshold) do
# Assume seconds since epoch
DateTime.from_unix!(threshold)
@ -228,11 +220,10 @@ defmodule AprsmeWeb.Live.Shared.PacketUtils do
def generate_callsign(packet) do
base_callsign = get_packet_field(packet, :base_callsign, "")
ssid = get_packet_field(packet, :ssid, nil)
if ssid != nil and ssid != "" do
"#{base_callsign}-#{ssid}"
else
base_callsign
end
format_callsign_with_ssid(base_callsign, ssid)
end
defp format_callsign_with_ssid(base_callsign, nil), do: base_callsign
defp format_callsign_with_ssid(base_callsign, ""), do: base_callsign
defp format_callsign_with_ssid(base_callsign, ssid), do: "#{base_callsign}-#{ssid}"
end

View file

@ -13,23 +13,17 @@ defmodule AprsmeWeb.Plugs.IPGeolocation do
def init(opts), do: opts
@impl true
def call(conn, _opts) do
# Only run for the main map page
if conn.request_path == "/" && conn.method == "GET" do
case get_session(conn, :ip_geolocation) do
nil ->
# No cached geolocation, fetch it
perform_geolocation(conn)
_cached ->
# Already have geolocation in session
conn
end
else
conn
end
def call(%{request_path: "/", method: "GET"} = conn, _opts) do
conn
|> get_session(:ip_geolocation)
|> handle_geolocation(conn)
end
def call(conn, _opts), do: conn
defp handle_geolocation(nil, conn), do: perform_geolocation(conn)
defp handle_geolocation(_cached, conn), do: conn
defp perform_geolocation(conn) do
ip = get_client_ip(conn)
Logger.info("IP geolocation: Starting lookup for IP #{ip}")
@ -133,16 +127,12 @@ defmodule AprsmeWeb.Plugs.IPGeolocation do
defp valid_ip_for_geolocation?(_), do: false
defp private_ip?(ip) do
cond do
String.starts_with?(ip, "127.") -> true
String.starts_with?(ip, "::1") -> true
String.starts_with?(ip, "10.") -> true
String.starts_with?(ip, "192.168.") -> true
String.starts_with?(ip, "172.") -> in_172_private_range?(ip)
true -> false
end
end
defp private_ip?("127." <> _), do: true
defp private_ip?("::1" <> _), do: true
defp private_ip?("10." <> _), do: true
defp private_ip?("192.168." <> _), do: true
defp private_ip?("172." <> _ = ip), do: in_172_private_range?(ip)
defp private_ip?(_), do: false
# Check if IP is in 172.16.0.0/12 range (172.16.0.0 - 172.31.255.255)
defp in_172_private_range?(ip) do

View file

@ -17,15 +17,14 @@ defmodule AprsmeWeb.Plugs.SetLocale do
end
defp get_locale_from_header(conn) do
case get_req_header(conn, "accept-language") do
[accept_language | _] ->
parse_accept_language(accept_language)
_ ->
nil
end
conn
|> get_req_header("accept-language")
|> extract_locale()
end
defp extract_locale([accept_language | _]), do: parse_accept_language(accept_language)
defp extract_locale(_), do: nil
defp parse_accept_language(accept_language) do
accept_language
|> String.split(",")

View file

@ -11,21 +11,11 @@ defmodule AprsmeWeb.WeatherUnits do
Returns :imperial for US, :metric for most other countries.
"""
@spec unit_system(String.t() | nil | any()) :: :imperial | :metric
def unit_system(locale) when is_binary(locale) do
case locale do
# Default to imperial for English
"en" -> :imperial
# Spanish-speaking countries mostly use metric
"es" -> :metric
# Germany uses metric
"de" -> :metric
# France uses metric
"fr" -> :metric
# Default to metric for unknown locales
_ -> :metric
end
end
def unit_system("en"), do: :imperial
def unit_system("es"), do: :metric
def unit_system("de"), do: :metric
def unit_system("fr"), do: :metric
def unit_system(locale) when is_binary(locale), do: :metric
def unit_system(nil), do: :imperial
def unit_system(_), do: :imperial
@ -34,12 +24,8 @@ defmodule AprsmeWeb.WeatherUnits do
Returns the temperature in the appropriate unit for the locale.
"""
@spec format_temperature(number() | any(), String.t()) :: {number() | any(), String.t()}
def format_temperature(temp, locale) when is_number(temp) do
case unit_system(locale) do
:imperial -> {temp, "°F"}
:metric -> {Convert.f_to_c(temp), "°C"}
end
end
def format_temperature(temp, locale) when is_number(temp),
do: format_by_unit(temp, locale, &Convert.f_to_c/1, "°F", "°C")
def format_temperature(temp, _locale), do: {temp, "°F"}
@ -48,12 +34,8 @@ defmodule AprsmeWeb.WeatherUnits do
Returns the wind speed in the appropriate unit for the locale.
"""
@spec format_wind_speed(number() | any(), String.t()) :: {number() | any(), String.t()}
def format_wind_speed(speed, locale) when is_number(speed) do
case unit_system(locale) do
:imperial -> {speed, "mph"}
:metric -> {Convert.mph_to_kph(speed), "km/h"}
end
end
def format_wind_speed(speed, locale) when is_number(speed),
do: format_by_unit(speed, locale, &Convert.mph_to_kph/1, "mph", "km/h")
def format_wind_speed(speed, _locale), do: {speed, "mph"}
@ -62,12 +44,8 @@ defmodule AprsmeWeb.WeatherUnits do
Returns the rain amount in the appropriate unit for the locale.
"""
@spec format_rain(number() | any(), String.t()) :: {number() | any(), String.t()}
def format_rain(rain, locale) when is_number(rain) do
case unit_system(locale) do
:imperial -> {rain, "in"}
:metric -> {Convert.inches_to_mm(rain), "mm"}
end
end
def format_rain(rain, locale) when is_number(rain),
do: format_by_unit(rain, locale, &Convert.inches_to_mm/1, "in", "mm")
def format_rain(rain, _locale), do: {rain, "in"}
@ -75,10 +53,7 @@ defmodule AprsmeWeb.WeatherUnits do
Formats pressure (keeps hPa for all locales as it's standard).
"""
@spec format_pressure(number() | any(), any()) :: {number() | any(), String.t()}
def format_pressure(pressure, _locale) when is_number(pressure) do
{pressure, "hPa"}
end
def format_pressure(pressure, _locale) when is_number(pressure), do: {pressure, "hPa"}
def format_pressure(pressure, _locale), do: {pressure, "hPa"}
@doc """
@ -90,23 +65,30 @@ defmodule AprsmeWeb.WeatherUnits do
rain: String.t(),
pressure: String.t()
}
def unit_labels(locale) do
case unit_system(locale) do
:imperial ->
%{
temperature: "°F",
wind_speed: "mph",
rain: "in",
pressure: "hPa"
}
def unit_labels(locale), do: do_unit_labels(unit_system(locale))
:metric ->
%{
temperature: "°C",
wind_speed: "km/h",
rain: "mm",
pressure: "hPa"
}
defp do_unit_labels(:imperial) do
%{
temperature: "°F",
wind_speed: "mph",
rain: "in",
pressure: "hPa"
}
end
defp do_unit_labels(:metric) do
%{
temperature: "°C",
wind_speed: "km/h",
rain: "mm",
pressure: "hPa"
}
end
defp format_by_unit(value, locale, converter, imperial_unit, metric_unit) do
case unit_system(locale) do
:imperial -> {value, imperial_unit}
:metric -> {converter.(value), metric_unit}
end
end
end