From 6061e02835282818d42b63946f140d5f869e6fef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 1 Nov 2025 21:01:07 +0000 Subject: [PATCH] Address code review feedback: fix Date object reuse and add clarifying comments Co-authored-by: THoguet <61246905+THoguet@users.noreply.github.com> --- src/app/components/calendar-month/calendar-month.ts | 3 +++ src/app/services/calendar-service.ts | 13 +++++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/app/components/calendar-month/calendar-month.ts b/src/app/components/calendar-month/calendar-month.ts index 331f269..72de621 100644 --- a/src/app/components/calendar-month/calendar-month.ts +++ b/src/app/components/calendar-month/calendar-month.ts @@ -19,6 +19,9 @@ export class CalendarMonth implements OnInit { year!: number; // Cache "now" as a class property to avoid recreating it on every call + // Note: This value is cached at component initialization and won't update if the day changes + // while the component is still mounted. This is acceptable for this use case since the app + // is primarily used within a single day session. private readonly now: Date = (() => { const date = new Date(); date.setHours(0, 0, 0, 0); diff --git a/src/app/services/calendar-service.ts b/src/app/services/calendar-service.ts index a58cabc..30839d7 100644 --- a/src/app/services/calendar-service.ts +++ b/src/app/services/calendar-service.ts @@ -101,6 +101,7 @@ export class CalendarService { // Cache holidays to avoid recalculation const currentYear = new Date().getFullYear(); if (this.holidayCacheYear === currentYear && this.holidayTimestampsCache) { + // Return timestamps directly for efficient lookup return Array.from(this.holidayTimestampsCache).map((ts) => new Date(ts)); } @@ -109,7 +110,7 @@ export class CalendarService { const holidays = hd.getHolidays(currentYear, lang); const holidaysPlusOne = hd.getHolidays(currentYear + 1, lang); holidays.push(...holidaysPlusOne); - let holidaysDate: Date[] = holidays.map((holiday) => new Date(holiday.date)); + const holidaysDate: Date[] = holidays.map((holiday) => new Date(holiday.date)); // Update cache this.holidayTimestampsCache = new Set(holidaysDate.map((d) => d.getTime())); @@ -736,19 +737,15 @@ export class SelectedDates implements SelectedDateInterface { this.datesSelected = this._datesSelected.slice(); this.grouping(); - // Mark this day as vacation - const startDay = new Date(day); - const endDay = new Date(day); - this.datesSelected.push( - new SelectedDate(vacType, new DateRange(startDay, endDay)), - ); + // Mark this day as vacation (reuse the day object since it's already normalized) + this.datesSelected.push(new SelectedDate(vacType, new DateRange(day, day))); const heuristic = this.computeHeuristics(); // Track only if better than current best if (heuristic > bestHeuristic) { bestHeuristic = heuristic; - const capturedDay = startDay; + const capturedDay = day; bestStrategy = { apply: () => { this._datesSelected = [...tempDatesBackup];