Skip to content

⚡ Bolt: [performance improvement] avoid intermediate array allocations in getGroupedFilmsByDate #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bartholomej wants to merge 1 commit into
base: master
Choose a base branch
from
+10 −8
Open

⚡ Bolt: [performance improvement] avoid intermediate array allocations in getGroupedFilmsByDate #177

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/helpers/cinema.helper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,16 @@ export const parseCinema = (el: HTMLElement | null): { city: string; name: strin

export const getGroupedFilmsByDate = (el: HTMLElement | null): CSFDCinemaGroupedFilmsByDate[] => {
const divs = el.querySelectorAll(':scope > div');
const getDatesAndFilms = divs
.map((_, index) => index)
.filter((index) => index % 2 === 0)
.map((index) => {
const [date, films] = divs.slice(index, index + 2);
const dateText = date?.firstChild?.textContent?.trim() ?? null;
return { date: dateText, films: getCinemaFilms('', films) };
});
const getDatesAndFilms: CSFDCinemaGroupedFilmsByDate[] = [];

// Performance optimization: Avoid intermediate array allocations (.map().filter().map())
// by using a single standard `for` loop with a step of 2.
for (let i = 0; i < divs.length; i += 2) {
const date = divs[i];
const films = divs[i + 1];
const dateText = date?.firstChild?.textContent?.trim() ?? null;
getDatesAndFilms.push({ date: dateText, films: getCinemaFilms('', films) });
}
Comment on lines +59 to +64
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard incomplete date/films pairs before calling getCinemaFilms.

With i < divs.length, an odd number of direct child divs makes films absent on the last iteration, and getCinemaFilms('', films) can crash because getCinemaFilms dereferences el. Keep the allocation-free loop, but skip incomplete pairs.

🛡️ Proposed fix 
-  for (let i = 0; i < divs.length; i += 2) {
+  for (let i = 0; i < divs.length - 1; i += 2) {
     const date = divs[i];
     const films = divs[i + 1];
-    const dateText = date?.firstChild?.textContent?.trim() ?? null;
+
+    if (!date || !films) continue;
+
+    const dateText = date.firstChild?.textContent?.trim() ?? null;
     getDatesAndFilms.push({ date: dateText, films: getCinemaFilms('', films) });
   }

As per coding guidelines, src/helpers/**/*.ts: “Never assume an element exists. CSFD changes layouts. Use optional chaining ?. or try/catch inside helpers for robust scraping.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (let i = 0; i < divs.length; i += 2) {
const date = divs[i];
const films = divs[i + 1];
const dateText = date?.firstChild?.textContent?.trim() ?? null;
getDatesAndFilms.push({ date: dateText, films: getCinemaFilms('', films) });
}
for (let i = 0; i < divs.length - 1; i += 2) {
const date = divs[i];
const films = divs[i + 1];
if (!date || !films) continue;
const dateText = date.firstChild?.textContent?.trim() ?? null;
getDatesAndFilms.push({ date: dateText, films: getCinemaFilms('', films) });
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/helpers/cinema.helper.ts` around lines 59 - 64, The loop in
cinema.helper.ts iterates by two but doesn't guard against an odd number of
divs, causing getCinemaFilms('', films) to be called with films undefined and
crash; update the for-loop that uses divs, date, and films to check that divs[i
+ 1] (films) exists before calling getCinemaFilms — if it's missing,
continue/skip that iteration (or use optional chaining to pass a safe value),
and ensure getDatesAndFilms only pushes when both dateText and films are
present; reference the loop variables divs, date, films and the getCinemaFilms
and getDatesAndFilms identifiers when making the change.


return getDatesAndFilms;
};
Expand Down
Loading