refactor: Phase 1 PR3/7 — Extract DataCollection IIFE module (#129)

[cheerc]

Summary

Extract 14 methods (11 public + 3 private helpers) from JavaScript.html to a new DataCollection.js.html IIFE module as part of #129 Phase 1.

Part of #129 Phase 1 (PR3/7)

Methods Extracted

Method	Type	Dependencies
_forEachCourse	private	none
_collectFromScheduleData	private	_forEachCourse
_collectFromAllCourses	private	none
getAllTags	public	_collectFromScheduleData
getGlobalAllTags	public	_collectFromAllCourses
getGlobalAllCourseNames	public	_collectFromAllCourses
getGlobalAllTeachers	public	_collectFromAllCourses
ensureDataIds	public	App.generateUniqueId (UtilityFunctions)
buildCourseColorMap	public	App.stringToHashCode, AppConfig.COURSE_COLORS
sortClassrooms	public	none
checkTimeConflict	public	App.timeToMinutes (UtilityFunctions)
countOccurrences	public	_forEachCourse
updateAllOccurrences	public	_forEachCourse
findNextUpcomingClasses	public	App.timeToMinutes, AppConfig.MODES

Changes

• DataCollection.js.html (new): IIFE module with 14 methods, all this.xxx → App.xxx
• JavaScript.html: Removed 14 methods (~208 lines), replaced with breadcrumb comments
• Index.html: Added DataCollection.js include after UtilityFunctions.js, before App.init()
• appWiringContracts.test.js: Moved 11 public + 3 private methods to IIFE_EXTRACTED set, updated count 41→30

Verification

• ✅ npm test: 1147/1147 pass (36 files)
• ✅ npx eslint --ext .html .: 0 errors (8 pre-existing warnings)
• ✅ Scope gate: 4 files changed
• ✅ Load order: JavaScript.html → UtilityFunctions.js → DataCollection.js → App.init()

Notable

• buildCourseColorMap default parameter dataSource = this.scheduleData changed to explicit undefined check since IIFE cannot reference this

[cheerc]

✅ VERIFIED

Reviewer: cb-team-reviewer
Reviewed HEAD: f6708ba65cca4845f0cd12a9ee754f14e475f894
Audit mode: D2 (1-hop import chain, correctness)
Skills: review-task-loop, ecc-code-reviewer

Files Reviewed

• DataCollection.js.html (231 lines, NEW) — IIFE module with 14 methods (11 public + 3 private)
• JavaScript.html (-203/+5 lines) — 14 methods removed, breadcrumb comments left
• Index.html (+1 line) — DataCollection include in load order
• tests/unit/appWiringContracts.test.js (±24 lines) — method reclassification, count 41→30

Summary

Phase 1 PR3/7: Extracts 14 data collection/transformation methods from JavaScript.html into DataCollection.js.html IIFE module. All this.xxx → App.xxx conversions complete.

this→App Conversion Verification (Critical Check)

Zero this. references in DataCollection.js.html — verified via grep.

Methods and their key conversions:

• _forEachCourse: no this. (pure iteration)
• _collectFromScheduleData: this._forEachCourse → App._forEachCourse
• _collectFromAllCourses: this.schedules → App.schedules
• getAllTags: this._collectFromScheduleData, this.scheduleData → App.
• getGlobalAllTags/CourseNames/Teachers: this._collectFromAllCourses → App.
• ensureDataIds: this.generateUniqueId → App.generateUniqueId (cross-domain: UtilityFunctions)
• buildCourseColorMap: see default param below; this.courseColorMap → App.courseColorMap, this.stringToHashCode → App.stringToHashCode
• sortClassrooms: no this. (pure function)
• checkTimeConflict: this.scheduleData → App.scheduleData, this.timeToMinutes → App.timeToMinutes
• countOccurrences/updateAllOccurrences: this._forEachCourse, this.scheduleData → App.
• findNextUpcomingClasses: this.nextUpcomingClassIds → App., this.currentViewMode/currentDayIndex → App., this.timeToMinutes → App.timeToMinutes

buildCourseColorMap Default Parameter Conversion (Dispatch Focus Item)

Original: buildCourseColorMap: function (dataSource = this.scheduleData)
Extracted: App.buildCourseColorMap = function(dataSource) { if (dataSource === undefined) dataSource = App.scheduleData; ... }

Behavior-equivalent ✅:

• ES6 default parameters trigger only on undefined (not null)
• === undefined check also triggers only on undefined
• Both evaluate App.scheduleData lazily at call time
• Cannot use default param syntax in IIFE because this would not refer to App

Cross-Domain Dependencies Verified (D2 1-hop)

• App.stringToHashCode (UtilityFunctions, PR1) — loaded before DataCollection ✅
• App.generateUniqueId (UtilityFunctions, PR1) — loaded before DataCollection ✅
• App.timeToMinutes (UtilityFunctions, PR1) — loaded before DataCollection ✅
• AppConfig.COURSE_COLORS, AppConfig.MODES — Config loads at L468, before DataCollection ✅

Load Order Verified

Config.js.html (L468) → AppConfig
JavaScript.html (L475) → App object + remaining methods
UtilityFunctions.js.html (L476) → PR1 (provides stringToHashCode, etc.)
DataCollection.js.html (L477) → PR3, receives App via IIFE
<script>App.init()</script> (L478)

Caller this.methodName() References

JavaScript.html L69 retains this.findNextUpcomingClasses() inside setInterval callback within App.init(). This is correct — this === App in that context, and findNextUpcomingClasses is attached to App via IIFE before init() runs.

Stage 1 — Correctness

Zero findings:

• 14/14 methods extracted (3 private + 11 public)
• No conflict markers
• Breadcrumb comments in JavaScript.html
• Public method count 41→30 (11 public DataCollection methods removed)
• IIFE_EXTRACTED set correctly updated with all 14 methods
• _forEachCourse internal inter-call works (both caller and callee use App.)
• Method bodies faithful to source (no logic changes except default param conversion)

Stage 2 — Adversarial

Zero findings:

• buildCourseColorMap default param conversion is the only non-trivial transformation — verified behavior-equivalent
• _forEachCourse is used by _collectFromScheduleData, countOccurrences, and updateAllOccurrences — all correctly use App._forEachCourse
• findNextUpcomingClasses references App.nextUpcomingClassIds (Set), App.currentViewMode, App.currentDayIndex, App.scheduleData, App.timeToMinutes — all App state/methods available after JavaScript.html + UtilityFunctions load
• No PR2 (LockManager) conflicts — independent method sets, verified parallel-safe

Evidence

• ran: gh pr view 147 --json headRefOid → f6708ba65cca4845f0cd12a9ee754f14e475f894 (SHA aligned)
• ran: grep 'this\.' DataCollection.js.html → NO this. references found
• ran: grep -n findNextUpcomingClasses JavaScript.html → L69 caller (this.findNextUpcomingClasses in init context), L690 breadcrumb
• ran: grep conflict-markers DataCollection.js.html → 0
• cited: DataCollection.js.html:L116-117 — if (dataSource === undefined) dataSource = App.scheduleData (default param conversion)
• cited: JavaScript.html:L65-69 — setInterval(() => { this.findNextUpcomingClasses(); ...}) (caller retains this., correct in App context)
• cited: Index.html:L477 — DataCollection.js loaded after UtilityFunctions.js, before init()