diff --git a/CODESMELL.md b/CODESMELL.md index 2cf1cf3..0004c96 100644 --- a/CODESMELL.md +++ b/CODESMELL.md @@ -241,15 +241,21 @@ --- -### CSM-014 — O(n²) Loops from `length/1` Inside Iteration -- **Files:** - - `lib/bds/publishing.ex:127` — `length(targets)` inside `Enum.reduce_while` (typically 3 targets, negligible impact) - - `lib/bds/rendering/list_archive.ex:299` — `index < length(grouped_blocks) - 1` inside `Enum.map` - - `lib/bds/generation/outputs.ex:216,230,232` — repeated `length(paginated_posts)` and `length(posts)` inside comprehensions - - `lib/bds/ui/sidebar.ex:556-565` — `acc.draft ++ [post]` in `Enum.reduce` (also covered by CSM-024) -- **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. -- **Fix:** Bind `total = length(list)` before the loop. For `acc ++ [item]`, use reverse-accumulate + `Enum.reverse`. -- **Test:** Run the function with a list of 1,000 items; assert it completes in linear time. +### ~~CSM-014 — O(n²) Loops from `length/1` Inside Iteration~~ ✅ FIXED +- **Fixed:** 2026-05-09 +- **What was done:** + - **`lib/bds/generation/outputs.ex`** — `build_category_outputs`: + - 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/generation/outputs.ex`** — `build_root_outputs`: + - Bound `total_items = length(posts)` before the loop, reused by `pagination_for_page`. Previously called `length(posts)` on every page iteration. + - **`lib/bds/generation/outputs.ex`** — `build_paginated_archive_outputs`: + - 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. --- diff --git a/lib/bds/generation/outputs.ex b/lib/bds/generation/outputs.ex index fac40a1..a04cdd4 100644 --- a/lib/bds/generation/outputs.ex +++ b/lib/bds/generation/outputs.ex @@ -207,14 +207,16 @@ defmodule BDS.Generation.Outputs do Enum.flat_map(posts_by_category, fn {category, posts} -> paginated_posts = Enum.chunk_every(posts, max(plan.max_posts_per_page, 1)) category_slug = archive_route_segment(category) + total_pages = length(paginated_posts) + total_items = length(posts) Enum.with_index(paginated_posts, 1) |> Enum.flat_map(fn {page_posts, page_number} -> Enum.map(languages, fn language -> pagination = %{ current_page: page_number, - total_pages: length(paginated_posts), - total_items: length(posts), + total_pages: total_pages, + total_items: total_items, items_per_page: max(plan.max_posts_per_page, 1), has_prev_page: page_number > 1, prev_page_href: @@ -227,9 +229,9 @@ defmodule BDS.Generation.Outputs do ), else: "" ), - has_next_page: page_number < length(paginated_posts), + has_next_page: page_number < total_pages, next_page_href: - if(page_number < length(paginated_posts), + if(page_number < total_pages, do: archive_href( 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()}] 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 |> paginate_posts(plan.max_posts_per_page) @@ -454,7 +457,7 @@ defmodule BDS.Generation.Outputs do pagination_for_page( page_number, total_pages, - length(posts), + total_items, plan.max_posts_per_page, route_language, [] @@ -468,7 +471,8 @@ defmodule BDS.Generation.Outputs do iodata())) :: [{String.t(), iodata()}] 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 |> paginate_posts(plan.max_posts_per_page) @@ -484,7 +488,7 @@ defmodule BDS.Generation.Outputs do pagination_for_page( page_number, total_pages, - length(posts), + total_items, plan.max_posts_per_page, route_language, segments diff --git a/lib/bds/publishing.ex b/lib/bds/publishing.ex index 84a427c..a510684 100644 --- a/lib/bds/publishing.ex +++ b/lib/bds/publishing.ex @@ -120,11 +120,13 @@ defmodule BDS.Publishing do defp run_upload(job_id, credentials, targets, uploader, report) do update_job(job_id, %{status: :running, error: nil}) + target_count = max(length(targets), 1) + result = Enum.with_index(targets, 1) |> Enum.reduce_while(:ok, fn {target, index}, :ok -> 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 :ok -> {:cont, :ok} diff --git a/lib/bds/rendering/list_archive.ex b/lib/bds/rendering/list_archive.ex index 710cc90..aebba8a 100644 --- a/lib/bds/rendering/list_archive.ex +++ b/lib/bds/rendering/list_archive.ex @@ -303,13 +303,15 @@ defmodule BDS.Rendering.ListArchive do ) |> Enum.sort_by(fn {label, _posts} -> label end) + last_index = length(grouped_blocks) - 1 + grouped_blocks |> Enum.with_index() |> Enum.map(fn {{date_label, grouped_posts}, index} -> %{ date_label: date_label, 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)) } end)