08. Code Review Report - khalillabban/Snorting-Code GitHub Wiki

Code Review Report

Release 1

PR #170 – Show additional information per building with pop-up

Comments: Accessibility support (screen readers), removal of unused styles, memoization of derived props, CI hygiene (ignore auto-generated SonarQube files), and cleaner conditional rendering.

PR #163 – Toggle between campus

Comments: Replace hardcoded values with shared constants, sync state with route params, and remove duplicated and overlapping tests.

PR #164 – Web map fixes and dependency updates

Comments: Scope control, removal of unnecessary scripts, avoid adding web-specific complexity to a mobile-only project.

PR #162 – View both campuses

Comments: Centralize styling in a shared theme, fix minor lifecycle logic issues.

PR #168 – Show current building

Comments: Resolve merge conflicts, ensure acceptance criteria coverage, add tests, and handle edge cases.


Release 2

PR #207 – Select the start and the end building

Comments: Apply the requested positioning and style updates for the campus toggle and location button to have a comfortable user experience for both Android and iOS.

PR #212 – Set current location as starting location

Comments: Improve the application's UX by transitioning location permissions to an opt-in model, implementing a distance threshold for building snapping, and ensuring that automatic updates do not overwrite manual user input.

PR #213 – Show outdoor directions on the map

Comments: Refactor route-fetching logic into a dedicated service, improve error handling and API key management, ensure polyline rendering reliability, and align implementation with acceptance criteria.

PR #217 – Choose methods of transportation

Comments: This PR requires UI and functional fixes to match the demo video, specifically regarding dynamic route styling, missing pins, and campus visibility, while also suggesting architectural cleanups and the addition of a "Current Location" navigation feature.

PR #251 – Location class

Comments: The feedback focuses on robustness and maintainability. Key recommendations include improving type safety (replacing any), adding error handling for JSON parsing, and extracting shared constants. It also suggests refining test coverage for edge cases, like URL parameters and all-day events, while cleaning up production logs.

PR #257 – Directions to next class

Comments: The reviewer suggests deriving nextClass from scheduleItems via useMemo to prevent "stale state." They also recommend adding a failure path for loadCachedSchedule to avoid silent failures and disabling actions when no classes exist.


Release 3

PR #278 – Indoor Map display

Comments: This PR focuses on improving code quality by cleaning up leftover "holdovers," optimizing performance through better variable reuse, and enhancing app stability with safer data parsing and UI constraints.

PR #289 – Navigation indoor map and locate room

Comments: This PR improves the robustness and performance of the navigation engine by implementing input validation for edge weights, fixing cross-floor and object-comparison logic bugs, and introducing lazy-loading for heavy SVG assets to significantly reduce app startup time and memory usage.

PR #311 – Navigation indoor map and locate room

Comments: This PR streamlines the indoor navigation UI by adding missing useEffect dependencies, optimizing useMemo hooks for point-of-interest (POI) filtering, and improving accessibility through better screen reader hints and property hidden attributes.

PR #312 – Navigation indoor and outdoor between SGW and Loyola

Comments: This PR prioritizes Type Safety and State Synchronization, specifically by replacing any casts with explicit interfaces for indoor-to-outdoor transition payloads, resolving missing useEffect dependencies, and introducing a more robust interactive route-step component.

PR #324 – Show nearest outdoor points of interest

Comments: This PR streamlines the outdoor POI feature by centralizing distance utilities and removing redundant search triggers, while also highlighting the need for better API error surfacing. DS_Store cleanup, and clear documentation for skipped hook dependencies.

PR #325 – Show direction to the selected nearest point of interest

Comments: This PR refines the point-of-interest selection logic by implementing placeId comparison for more reliable state updates and includes a minor cleanup of redundant comments.

Code Review Process Evolution

Throughout the project, our code review process became significantly more structured and effective.

In earlier releases, comments were primarily focused on identifying obvious issues such as missing tests, minor bugs, and basic code quality improvements. As the project progressed, reviews became more thorough and deliberate. Reviewers increasingly provided concrete suggestions with code snippets, improving clarity and making feedback easier to act on.

We also began engaging in more back-and-forth discussions within pull request comments. These discussions helped justify design decisions, challenge unnecessary changes, and align on best practices across the team. This shift turned code reviews into a collaborative process rather than a simple approval step.

Towards the end of the project, as development shifted to smaller bug fixes and UI refinements, the volume of review comments naturally decreased. Many changes were straightforward and low-risk, leading to faster approvals and merges while still maintaining confidence in code quality.