fix(perf): bind length/1 to variables before loop bodies in route paths and sidebar (CSM-029)
This commit is contained in:
12
CODESMELL.md
12
CODESMELL.md
@@ -434,10 +434,14 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### CSM-029 — `length/1` in Guards or Comparisons
|
### ~~CSM-029 — `length/1` in Guards or Comparisons~~ ✅ FIXED
|
||||||
- **Files:** `lib/bds/generation/outputs.ex`, `lib/bds/ui/sidebar.ex`
|
- **Fixed:** 2026-05-11
|
||||||
- **What:** `length(list)` is O(n). Using it inside a loop makes the whole loop O(n²).
|
- **What was done:**
|
||||||
- **Fix:** Bind the length before the loop.
|
- **`lib/bds/generation/outputs.ex`** — `category_route_paths`, `tag_route_paths`, `date_route_paths`:
|
||||||
|
- Bound `post_count = length(posts)` at the start of each `Enum.flat_map` callback, before passing to `paginated_archive_paths`. Eliminates inline `length/1` calls inside loop bodies.
|
||||||
|
- **`lib/bds/ui/sidebar.ex`** — `build_post_section`:
|
||||||
|
- Bound `post_count = length(posts)` before the map literal instead of computing `length(posts)` inline in the `count:` field.
|
||||||
|
- Added 3 source-level tests in `test/bds/csm029_length_in_guards_test.exs`: no inline `length(posts)` in `paginated_archive_paths` calls, no inline `length()` in `build_post_section` map literal, no inline `length(posts)` in route path function callbacks.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -80,10 +80,12 @@ defmodule BDS.Generation.Outputs do
|
|||||||
def category_route_paths(plan, posts_by_category, route_language) do
|
def category_route_paths(plan, posts_by_category, route_language) do
|
||||||
if :category in plan.sections do
|
if :category in plan.sections do
|
||||||
Enum.flat_map(posts_by_category, fn {category, posts} ->
|
Enum.flat_map(posts_by_category, fn {category, posts} ->
|
||||||
|
post_count = length(posts)
|
||||||
|
|
||||||
paginated_archive_paths(
|
paginated_archive_paths(
|
||||||
route_language,
|
route_language,
|
||||||
["category", archive_route_segment(category)],
|
["category", archive_route_segment(category)],
|
||||||
length(posts),
|
post_count,
|
||||||
plan.max_posts_per_page
|
plan.max_posts_per_page
|
||||||
)
|
)
|
||||||
end)
|
end)
|
||||||
@@ -96,10 +98,12 @@ defmodule BDS.Generation.Outputs do
|
|||||||
def tag_route_paths(plan, posts_by_tag, route_language) do
|
def tag_route_paths(plan, posts_by_tag, route_language) do
|
||||||
if :tag in plan.sections do
|
if :tag in plan.sections do
|
||||||
Enum.flat_map(posts_by_tag, fn {tag, posts} ->
|
Enum.flat_map(posts_by_tag, fn {tag, posts} ->
|
||||||
|
post_count = length(posts)
|
||||||
|
|
||||||
paginated_archive_paths(
|
paginated_archive_paths(
|
||||||
route_language,
|
route_language,
|
||||||
["tag", archive_route_segment(tag)],
|
["tag", archive_route_segment(tag)],
|
||||||
length(posts),
|
post_count,
|
||||||
plan.max_posts_per_page
|
plan.max_posts_per_page
|
||||||
)
|
)
|
||||||
end)
|
end)
|
||||||
@@ -113,10 +117,12 @@ defmodule BDS.Generation.Outputs do
|
|||||||
if :date in plan.sections do
|
if :date in plan.sections do
|
||||||
year_paths =
|
year_paths =
|
||||||
Enum.flat_map(post_index.posts_by_year, fn {year, posts} ->
|
Enum.flat_map(post_index.posts_by_year, fn {year, posts} ->
|
||||||
|
post_count = length(posts)
|
||||||
|
|
||||||
paginated_archive_paths(
|
paginated_archive_paths(
|
||||||
route_language,
|
route_language,
|
||||||
[Integer.to_string(year)],
|
[Integer.to_string(year)],
|
||||||
length(posts),
|
post_count,
|
||||||
plan.max_posts_per_page
|
plan.max_posts_per_page
|
||||||
)
|
)
|
||||||
end)
|
end)
|
||||||
@@ -124,11 +130,12 @@ defmodule BDS.Generation.Outputs do
|
|||||||
month_paths =
|
month_paths =
|
||||||
Enum.flat_map(post_index.posts_by_year_month, fn {year_month, posts} ->
|
Enum.flat_map(post_index.posts_by_year_month, fn {year_month, posts} ->
|
||||||
[year, month] = String.split(year_month, "/", parts: 2)
|
[year, month] = String.split(year_month, "/", parts: 2)
|
||||||
|
post_count = length(posts)
|
||||||
|
|
||||||
paginated_archive_paths(
|
paginated_archive_paths(
|
||||||
route_language,
|
route_language,
|
||||||
[year, month],
|
[year, month],
|
||||||
length(posts),
|
post_count,
|
||||||
plan.max_posts_per_page
|
plan.max_posts_per_page
|
||||||
)
|
)
|
||||||
end)
|
end)
|
||||||
@@ -136,11 +143,12 @@ defmodule BDS.Generation.Outputs do
|
|||||||
day_paths =
|
day_paths =
|
||||||
Enum.flat_map(post_index.posts_by_year_month_day, fn {year_month_day, posts} ->
|
Enum.flat_map(post_index.posts_by_year_month_day, fn {year_month_day, posts} ->
|
||||||
[year, month, day] = String.split(year_month_day, "/", parts: 3)
|
[year, month, day] = String.split(year_month_day, "/", parts: 3)
|
||||||
|
post_count = length(posts)
|
||||||
|
|
||||||
paginated_archive_paths(
|
paginated_archive_paths(
|
||||||
route_language,
|
route_language,
|
||||||
[year, month, day],
|
[year, month, day],
|
||||||
length(posts),
|
post_count,
|
||||||
plan.max_posts_per_page
|
plan.max_posts_per_page
|
||||||
)
|
)
|
||||||
end)
|
end)
|
||||||
|
|||||||
@@ -821,11 +821,13 @@ defmodule BDS.UI.Sidebar do
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
defp build_post_section(title, status, posts, translation_counts, published_meta?) do
|
defp build_post_section(title, status, posts, translation_counts, published_meta?) do
|
||||||
|
post_count = length(posts)
|
||||||
|
|
||||||
%{
|
%{
|
||||||
id: Atom.to_string(status),
|
id: Atom.to_string(status),
|
||||||
title: title,
|
title: title,
|
||||||
status: Atom.to_string(status),
|
status: Atom.to_string(status),
|
||||||
count: length(posts),
|
count: post_count,
|
||||||
items:
|
items:
|
||||||
Enum.map(posts, fn post ->
|
Enum.map(posts, fn post ->
|
||||||
%{
|
%{
|
||||||
|
|||||||
55
test/bds/csm029_length_in_guards_test.exs
Normal file
55
test/bds/csm029_length_in_guards_test.exs
Normal file
@@ -0,0 +1,55 @@
|
|||||||
|
defmodule BDS.CSM029LengthInGuardsTest do
|
||||||
|
use ExUnit.Case, async: true
|
||||||
|
|
||||||
|
@outputs_path "lib/bds/generation/outputs.ex"
|
||||||
|
@sidebar_path "lib/bds/ui/sidebar.ex"
|
||||||
|
|
||||||
|
describe "source-level: length/1 bound before use in loops" do
|
||||||
|
test "outputs.ex route path functions bind length before passing to paginated_archive_paths" do
|
||||||
|
source = File.read!(@outputs_path)
|
||||||
|
|
||||||
|
refute source =~ ~r/paginated_archive_paths\([^)]*length\(posts\)/,
|
||||||
|
"length(posts) should be bound to a variable before passing to paginated_archive_paths"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "sidebar.ex build_post_section binds length before map literal" do
|
||||||
|
source = File.read!(@sidebar_path)
|
||||||
|
|
||||||
|
lines =
|
||||||
|
source
|
||||||
|
|> String.split("\n")
|
||||||
|
|> Enum.with_index(1)
|
||||||
|
|
||||||
|
build_section_lines =
|
||||||
|
Enum.filter(lines, fn {line, _} -> line =~ "defp build_post_section" end)
|
||||||
|
|
||||||
|
for {_line, line_no} <- build_section_lines do
|
||||||
|
context =
|
||||||
|
lines
|
||||||
|
|> Enum.drop(line_no - 1)
|
||||||
|
|> Enum.take(30)
|
||||||
|
|> Enum.map(fn {l, _} -> l end)
|
||||||
|
|> Enum.join("\n")
|
||||||
|
|
||||||
|
refute context =~ ~r/count: length\(/,
|
||||||
|
"length() should be pre-bound, not inline in map literal"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
test "no length/1 calls remain inline inside Enum.flat_map callbacks in route path functions" do
|
||||||
|
source = File.read!(@outputs_path)
|
||||||
|
|
||||||
|
route_fns = ["category_route_paths", "tag_route_paths", "date_route_paths"]
|
||||||
|
|
||||||
|
for fn_name <- route_fns do
|
||||||
|
fn_match = Regex.run(~r/def #{fn_name}\(.*?\n end/s, source)
|
||||||
|
assert fn_match, "Expected to find #{fn_name} in outputs.ex"
|
||||||
|
|
||||||
|
[fn_body] = fn_match
|
||||||
|
|
||||||
|
refute fn_body =~ ~r/\blength\(posts\)(?!\s)/,
|
||||||
|
"#{fn_name} should use pre-bound post_count instead of inline length(posts)"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Reference in New Issue
Block a user