From 012899792e44e89427cba28061c93ef1aab27f2c Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Fri, 1 May 2026 13:47:20 -0500 Subject: [PATCH] cleanup --- results.md | 93 ------------------------------------------------------ 1 file changed, 93 deletions(-) delete mode 100644 results.md diff --git a/results.md b/results.md deleted file mode 100644 index b4078def..00000000 --- a/results.md +++ /dev/null @@ -1,93 +0,0 @@ -# Codebase Optimization & Refactoring Review - -## 1. Module Decomposition - -Several context modules have grown beyond manageable sizes, making them difficult to navigate and maintain. - -* **`Towerops.Snmp` (~3000 lines)**: This module is acting as a "God Context" for all things SNMP. - * **Recommendation**: Split into sub-contexts like `Towerops.Snmp.Discovery`, `Towerops.Snmp.Queries`, and `Towerops.Snmp.Monitoring`. -* **`Towerops.Accounts` (~1900 lines)**: Handles everything from user registration to TOTP device management. - * **Recommendation**: Move TOTP logic to `Towerops.Accounts.TOTP` and session management to `Towerops.Accounts.Sessions`. -* **`Towerops.Topology` (~1600 lines)**: Contains complex graph inference logic alongside standard CRUD. - * **Recommendation**: Extract the inference engine logic into `Towerops.Topology.InferenceEngine`. - -## 2. DRY Refactoring Opportunities - -### SNMP Check Creation -In `Towerops.Snmp.create_checks_from_discovery`, there is a repetitive pattern for creating checks for sensors, interfaces, processors, and storage. - -```elixir -# Current Pattern -results = Enum.reduce(snmp_device.sensors, results, fn sensor, acc -> - case create_sensor_check(device, sensor) do - {:ok, _check} -> Map.update!(acc, :sensors, &(&1 + 1)) - {:error, reason} -> Map.update!(acc, :errors, &[{:sensor, sensor.id, reason} | &1]) - end -end) -``` - -**Recommendation**: Create a generic `create_discovered_checks(device, entities, type, check_fun)` helper to reduce boilerplate. - -### Topology Evidence Collection -`collect_lldp_evidence`, `collect_cdp_evidence`, `collect_mac_evidence`, and `collect_arp_evidence` all perform similar transformations. - -**Recommendation**: Standardize the evidence collection pipeline. Use a common internal function that takes a query and a transformation mapper. - -### Ecto Query Repetition -Many modules repeat basic query logic like `where(organization_id: ^org_id)`. - -**Recommendation**: Implement "Query" modules for each schema (e.g., `Towerops.Devices.DeviceQuery`) containing reusable query fragments that can be composed. - -## 3. Functional Pattern Matching vs. Conditionals - -### Replace `cond` with Pattern Matching -In `Towerops.Topology.infer_role_from_capabilities`, a `cond` block is used to match strings in a list. - -**Recommendation**: Use multi-clause functions or a more declarative approach. -```elixir -defp role_from_cap(["router" | _]), do: :router -defp role_from_cap(["bridge", "wlan-ap" | _]), do: :wireless -# ... -``` - -### Avoid `if` for Optional Query Filters -In `Towerops.Snmp.get_sensor_readings`, an `if` statement is used to conditionally add a `where` clause. - -**Recommendation**: Use a pipe-friendly helper: -```elixir -defp maybe_filter_since(query, nil), do: query -defp maybe_filter_since(query, since), do: where(query, [r], r.checked_at >= ^since) - -# Usage -query -|> maybe_filter_since(since) -|> Repo.all() -``` - -## 4. Performance Optimizations - -### Batch Database Operations -In `Towerops.Topology.upsert_link`, evidence records are inserted one-by-one in an `Enum.each` loop. - -**Recommendation**: Use `Repo.insert_all` to insert all evidence records in a single round-trip to the database. - -### Preloading Strategy -Some functions perform manual preloads inside loops or deep in the call stack. - -**Recommendation**: Ensure associations are preloaded at the edge of the context (the public API) to avoid N+1 queries and redundant database calls. - -## 5. Human-Centric Refactoring - -### Break Down Large Functions -Functions like `Towerops.Topology.build_device_lookup` are doing too much in one block. - -**Recommendation**: Break these into smaller, named stages: -1. `fetch_lookup_devices(org_id)` -2. `map_ips_to_ids(devices)` -3. `map_names_to_ids(devices)` -4. `map_macs_to_ids(devices)` - -### Consistent Result Handling -While `Towerops.Result` exists, it isn't used everywhere. - -**Recommendation**: Standardize on `{:ok, value} | {:error, reason}` across all context boundaries and use `Towerops.Result` helpers to compose operations.