From bdb0dd8c38ef9b34f6820e0464d19a0c61c011ed Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 1 Jul 2026 18:04:36 -0500 Subject: [PATCH] =?UTF-8?q?refactor:=20apply=20FP=20patterns=20=E2=80=94?= =?UTF-8?q?=20with-chains=20+=20multi-clause=20over=20if/case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - accounts.ex: convert get_user_by_email_and_password if→with-chain with explicit User struct match and else→nil. Flatten delete_account nested if (password check + sole-owner check) into single with-chain with named else clauses, eliminating do_delete_account indirection. - organizations.ex: flatten update_member_role nested if/case into with-chain — parse_role guard → membership fetch → owner check, each with explicit else branch. Prevents variable shadowing by using current_role for the membership pattern match. - capacity.ex: extract calculate_throughput_from_stats inner if into multi-clause compute_throughput_from_pairs/1 — empty list clause returns zero_throughput(), non-empty clause handles the computation. --- lib/towerops/accounts.ex | 25 +++++++++------------- lib/towerops/capacity.ex | 40 ++++++++++++++++++----------------- lib/towerops/organizations.ex | 34 +++++++++++++---------------- 3 files changed, 46 insertions(+), 53 deletions(-) diff --git a/lib/towerops/accounts.ex b/lib/towerops/accounts.ex index 4527b5dc..1e02eaf3 100644 --- a/lib/towerops/accounts.ex +++ b/lib/towerops/accounts.ex @@ -56,8 +56,12 @@ defmodule Towerops.Accounts do """ @spec get_user_by_email_and_password(String.t(), String.t()) :: User.t() | nil def get_user_by_email_and_password(email, password) when is_binary(email) and is_binary(password) do - user = Repo.get_by(User, email: email) - if User.valid_password?(user, password), do: user + with %User{} = user <- Repo.get_by(User, email: email), + true <- User.valid_password?(user, password) do + user + else + _ -> nil + end end @doc """ @@ -476,21 +480,12 @@ defmodule Towerops.Accounts do | {:error, :invalid_password} | {:error, :sole_owner, [String.t()]} def delete_account(%User{} = user, password) when is_binary(password) do - if User.valid_password?(user, password) do - do_delete_account(user) - else - {:error, :invalid_password} - end - end - - defp do_delete_account(user) do - sole_owner_orgs = sole_owner_organizations(user.id) - - if sole_owner_orgs == [] do + with true <- User.valid_password?(user, password), + [] <- sole_owner_organizations(user.id) do delete_account_transaction(user) else - org_names = Enum.map(sole_owner_orgs, & &1.name) - {:error, :sole_owner, org_names} + false -> {:error, :invalid_password} + orgs when is_list(orgs) -> {:error, :sole_owner, Enum.map(orgs, & &1.name)} end end diff --git a/lib/towerops/capacity.ex b/lib/towerops/capacity.ex index ed90b653..e788638d 100644 --- a/lib/towerops/capacity.ex +++ b/lib/towerops/capacity.ex @@ -211,28 +211,30 @@ defmodule Towerops.Capacity do # Filter out zero-bps pairs (from clamped negative deltas / anomalies) valid_pairs = Enum.reject(pairs, fn {in_bps, out_bps} -> in_bps == 0.0 and out_bps == 0.0 end) - if Enum.empty?(valid_pairs) do - zero_throughput() - else - in_values = Enum.map(valid_pairs, &elem(&1, 0)) - out_values = Enum.map(valid_pairs, &elem(&1, 1)) + compute_throughput_from_pairs(valid_pairs) + end - avg_in = Enum.sum(in_values) / length(in_values) - avg_out = Enum.sum(out_values) / length(out_values) + defp compute_throughput_from_pairs([]), do: zero_throughput() - # Peak values for capacity planning (95th percentile approximation) - peak_in = percentile(in_values, 95) - peak_out = percentile(out_values, 95) + defp compute_throughput_from_pairs(valid_pairs) do + in_values = Enum.map(valid_pairs, &elem(&1, 0)) + out_values = Enum.map(valid_pairs, &elem(&1, 1)) - %{ - in_bps: Float.round(avg_in, 2), - out_bps: Float.round(avg_out, 2), - max_bps: Float.round(max(avg_in, avg_out), 2), - peak_in_bps: Float.round(peak_in, 2), - peak_out_bps: Float.round(peak_out, 2), - peak_bps: Float.round(max(peak_in, peak_out), 2) - } - end + avg_in = Enum.sum(in_values) / length(in_values) + avg_out = Enum.sum(out_values) / length(out_values) + + # Peak values for capacity planning (95th percentile approximation) + peak_in = percentile(in_values, 95) + peak_out = percentile(out_values, 95) + + %{ + in_bps: Float.round(avg_in, 2), + out_bps: Float.round(avg_out, 2), + max_bps: Float.round(max(avg_in, avg_out), 2), + peak_in_bps: Float.round(peak_in, 2), + peak_out_bps: Float.round(peak_out, 2), + peak_bps: Float.round(max(peak_in, peak_out), 2) + } end # 400 Gbps — values above this are counter anomalies, not real traffic diff --git a/lib/towerops/organizations.ex b/lib/towerops/organizations.ex index 6d7753d8..29bcdc98 100644 --- a/lib/towerops/organizations.ex +++ b/lib/towerops/organizations.ex @@ -264,28 +264,24 @@ defmodule Towerops.Organizations do Updates a member's role. Owners cannot have their role changed. """ def update_member_role(organization_id, user_id, new_role) do - role_atom = parse_role(new_role) - - if role_atom == :invalid do - changeset = - %Membership{} - |> Ecto.Changeset.change() - |> Ecto.Changeset.add_error(:role, "is invalid") - - {:error, changeset} + with new_role when new_role != :invalid <- parse_role(new_role), + %Membership{} = membership <- get_membership(organization_id, user_id), + %{role: current_role} when current_role != :owner <- membership do + membership + |> Membership.role_update_changeset(new_role) + |> Repo.update() else - case get_membership(organization_id, user_id) do - %Membership{role: :owner} -> - {:error, :cannot_change_owner_role} + :invalid -> + {:error, + %Membership{} + |> Ecto.Changeset.change() + |> Ecto.Changeset.add_error(:role, "is invalid")} - %Membership{} = membership -> - membership - |> Membership.role_update_changeset(role_atom) - |> Repo.update() + %Membership{role: :owner} -> + {:error, :cannot_change_owner_role} - nil -> - {:error, :not_found} - end + nil -> + {:error, :not_found} end end