fix: fixed CSM-014
This commit is contained in:
24
CODESMELL.md
24
CODESMELL.md
@@ -241,15 +241,21 @@
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
### CSM-014 — O(n²) Loops from `length/1` Inside Iteration
|
### ~~CSM-014 — O(n²) Loops from `length/1` Inside Iteration~~ ✅ FIXED
|
||||||
- **Files:**
|
- **Fixed:** 2026-05-09
|
||||||
- `lib/bds/publishing.ex:127` — `length(targets)` inside `Enum.reduce_while` (typically 3 targets, negligible impact)
|
- **What was done:**
|
||||||
- `lib/bds/rendering/list_archive.ex:299` — `index < length(grouped_blocks) - 1` inside `Enum.map`
|
- **`lib/bds/generation/outputs.ex`** — `build_category_outputs`:
|
||||||
- `lib/bds/generation/outputs.ex:216,230,232` — repeated `length(paginated_posts)` and `length(posts)` inside comprehensions
|
- Bound `total_pages = length(paginated_posts)` and `total_items = length(posts)` before the nested loop. Previously called `length/1` 4 times per page × language iteration.
|
||||||
- `lib/bds/ui/sidebar.ex:556-565` — `acc.draft ++ [post]` in `Enum.reduce` (also covered by CSM-024)
|
- **`lib/bds/generation/outputs.ex`** — `build_root_outputs`:
|
||||||
- **Note:** In `publishing.ex` the O(n²) is negligible (3 targets). The real impact is in `outputs.ex` and `list_archive.ex` with large post sets.
|
- Bound `total_items = length(posts)` before the loop, reused by `pagination_for_page`. Previously called `length(posts)` on every page iteration.
|
||||||
- **Fix:** Bind `total = length(list)` before the loop. For `acc ++ [item]`, use reverse-accumulate + `Enum.reverse`.
|
- **`lib/bds/generation/outputs.ex`** — `build_paginated_archive_outputs`:
|
||||||
- **Test:** Run the function with a list of 1,000 items; assert it completes in linear time.
|
- Bound `total_items = length(posts)` before the loop. Previously called `length(posts)` inside the nested page × language loop.
|
||||||
|
- **`lib/bds/rendering/list_archive.ex`** — `build_day_blocks`:
|
||||||
|
- Bound `last_index = length(grouped_blocks) - 1` before the `Enum.map`. Previously called `length(grouped_blocks)` on every iteration.
|
||||||
|
- **`lib/bds/publishing.ex`** — `run_upload`:
|
||||||
|
- Bound `target_count = max(length(targets), 1)` before the `Enum.reduce_while`. Negligible impact (3 targets) but fixed for consistency.
|
||||||
|
- `lib/bds/ui/sidebar.ex` `acc.draft ++ [post]` was already fixed by CSM-005 (replaced with `Enum.group_by`).
|
||||||
|
- Added 3 tests in `test/bds/csm014_length_in_loop_test.exs`: multi-page pagination correctness, single-page pagination correctness, 1000-post linear time completion.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -207,14 +207,16 @@ defmodule BDS.Generation.Outputs do
|
|||||||
Enum.flat_map(posts_by_category, fn {category, posts} ->
|
Enum.flat_map(posts_by_category, fn {category, posts} ->
|
||||||
paginated_posts = Enum.chunk_every(posts, max(plan.max_posts_per_page, 1))
|
paginated_posts = Enum.chunk_every(posts, max(plan.max_posts_per_page, 1))
|
||||||
category_slug = archive_route_segment(category)
|
category_slug = archive_route_segment(category)
|
||||||
|
total_pages = length(paginated_posts)
|
||||||
|
total_items = length(posts)
|
||||||
|
|
||||||
Enum.with_index(paginated_posts, 1)
|
Enum.with_index(paginated_posts, 1)
|
||||||
|> Enum.flat_map(fn {page_posts, page_number} ->
|
|> Enum.flat_map(fn {page_posts, page_number} ->
|
||||||
Enum.map(languages, fn language ->
|
Enum.map(languages, fn language ->
|
||||||
pagination = %{
|
pagination = %{
|
||||||
current_page: page_number,
|
current_page: page_number,
|
||||||
total_pages: length(paginated_posts),
|
total_pages: total_pages,
|
||||||
total_items: length(posts),
|
total_items: total_items,
|
||||||
items_per_page: max(plan.max_posts_per_page, 1),
|
items_per_page: max(plan.max_posts_per_page, 1),
|
||||||
has_prev_page: page_number > 1,
|
has_prev_page: page_number > 1,
|
||||||
prev_page_href:
|
prev_page_href:
|
||||||
@@ -227,9 +229,9 @@ defmodule BDS.Generation.Outputs do
|
|||||||
),
|
),
|
||||||
else: ""
|
else: ""
|
||||||
),
|
),
|
||||||
has_next_page: page_number < length(paginated_posts),
|
has_next_page: page_number < total_pages,
|
||||||
next_page_href:
|
next_page_href:
|
||||||
if(page_number < length(paginated_posts),
|
if(page_number < total_pages,
|
||||||
do:
|
do:
|
||||||
archive_href(
|
archive_href(
|
||||||
route_language(plan.language, language),
|
route_language(plan.language, language),
|
||||||
@@ -436,7 +438,8 @@ defmodule BDS.Generation.Outputs do
|
|||||||
|
|
||||||
@spec build_root_outputs(map(), String.t(), [map()]) :: [{String.t(), iodata()}]
|
@spec build_root_outputs(map(), String.t(), [map()]) :: [{String.t(), iodata()}]
|
||||||
def build_root_outputs(plan, language, posts) do
|
def build_root_outputs(plan, language, posts) do
|
||||||
total_pages = page_count(length(posts), plan.max_posts_per_page)
|
total_items = length(posts)
|
||||||
|
total_pages = page_count(total_items, plan.max_posts_per_page)
|
||||||
|
|
||||||
posts
|
posts
|
||||||
|> paginate_posts(plan.max_posts_per_page)
|
|> paginate_posts(plan.max_posts_per_page)
|
||||||
@@ -454,7 +457,7 @@ defmodule BDS.Generation.Outputs do
|
|||||||
pagination_for_page(
|
pagination_for_page(
|
||||||
page_number,
|
page_number,
|
||||||
total_pages,
|
total_pages,
|
||||||
length(posts),
|
total_items,
|
||||||
plan.max_posts_per_page,
|
plan.max_posts_per_page,
|
||||||
route_language,
|
route_language,
|
||||||
[]
|
[]
|
||||||
@@ -468,7 +471,8 @@ defmodule BDS.Generation.Outputs do
|
|||||||
iodata())) ::
|
iodata())) ::
|
||||||
[{String.t(), iodata()}]
|
[{String.t(), iodata()}]
|
||||||
def build_paginated_archive_outputs(plan, languages, segments, posts, render_fun) do
|
def build_paginated_archive_outputs(plan, languages, segments, posts, render_fun) do
|
||||||
total_pages = page_count(length(posts), plan.max_posts_per_page)
|
total_items = length(posts)
|
||||||
|
total_pages = page_count(total_items, plan.max_posts_per_page)
|
||||||
|
|
||||||
posts
|
posts
|
||||||
|> paginate_posts(plan.max_posts_per_page)
|
|> paginate_posts(plan.max_posts_per_page)
|
||||||
@@ -484,7 +488,7 @@ defmodule BDS.Generation.Outputs do
|
|||||||
pagination_for_page(
|
pagination_for_page(
|
||||||
page_number,
|
page_number,
|
||||||
total_pages,
|
total_pages,
|
||||||
length(posts),
|
total_items,
|
||||||
plan.max_posts_per_page,
|
plan.max_posts_per_page,
|
||||||
route_language,
|
route_language,
|
||||||
segments
|
segments
|
||||||
|
|||||||
@@ -120,11 +120,13 @@ defmodule BDS.Publishing do
|
|||||||
defp run_upload(job_id, credentials, targets, uploader, report) do
|
defp run_upload(job_id, credentials, targets, uploader, report) do
|
||||||
update_job(job_id, %{status: :running, error: nil})
|
update_job(job_id, %{status: :running, error: nil})
|
||||||
|
|
||||||
|
target_count = max(length(targets), 1)
|
||||||
|
|
||||||
result =
|
result =
|
||||||
Enum.with_index(targets, 1)
|
Enum.with_index(targets, 1)
|
||||||
|> Enum.reduce_while(:ok, fn {target, index}, :ok ->
|
|> Enum.reduce_while(:ok, fn {target, index}, :ok ->
|
||||||
files = list_target_files(target)
|
files = list_target_files(target)
|
||||||
report.(index / max(length(targets), 1), "Uploading #{target.kind}")
|
report.(index / target_count, "Uploading #{target.kind}")
|
||||||
|
|
||||||
case uploader.(target, files, credentials) do
|
case uploader.(target, files, credentials) do
|
||||||
:ok -> {:cont, :ok}
|
:ok -> {:cont, :ok}
|
||||||
|
|||||||
@@ -303,13 +303,15 @@ defmodule BDS.Rendering.ListArchive do
|
|||||||
)
|
)
|
||||||
|> Enum.sort_by(fn {label, _posts} -> label end)
|
|> Enum.sort_by(fn {label, _posts} -> label end)
|
||||||
|
|
||||||
|
last_index = length(grouped_blocks) - 1
|
||||||
|
|
||||||
grouped_blocks
|
grouped_blocks
|
||||||
|> Enum.with_index()
|
|> Enum.with_index()
|
||||||
|> Enum.map(fn {{date_label, grouped_posts}, index} ->
|
|> Enum.map(fn {{date_label, grouped_posts}, index} ->
|
||||||
%{
|
%{
|
||||||
date_label: date_label,
|
date_label: date_label,
|
||||||
show_date_marker: true,
|
show_date_marker: true,
|
||||||
show_separator: index < length(grouped_blocks) - 1,
|
show_separator: index < last_index,
|
||||||
posts: Enum.sort_by(grouped_posts, &Map.get(&1, :created_at))
|
posts: Enum.sort_by(grouped_posts, &Map.get(&1, :created_at))
|
||||||
}
|
}
|
||||||
end)
|
end)
|
||||||
|
|||||||
Reference in New Issue
Block a user