Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors code to improve readability and performance by using modern Swift syntax and more efficient data structures.
- Replaced explicit
returnstatements with implicit returns for single-expression properties and switch cases - Refactored complex nested filtering logic into a cleaner, more maintainable implementation
- Optimized vote storage to use Set-based operations instead of Array operations
- Removed unused commented-out code file
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Sources/Roadmap/Styling/RoadmapTemplates.swift | Simplified switch cases and conditional blocks with implicit returns |
| Sources/Roadmap/Styling/RoadmapStyle.swift | Removed explicit return statements in conditional property |
| Sources/Roadmap/RoadmapVoteButton.swift | Consolidated accessibility value formatting logic into a single expression |
| Sources/Roadmap/RoadmapViewModel.swift | Refactored filtering logic from nested conditionals to a unified filter expression |
| Sources/Roadmap/RoadmapView.swift | Simplified computed properties with implicit returns |
| Sources/Roadmap/Models/RoadmapFeature.swift | Replaced array-based vote storage with Set-based operations for better performance |
| Sources/Roadmap/Extensions/ColorExtensions.swift | Removed explicit return statements in conditional property |
| Sources/Roadmap/Extensions/ArrayExtensions.swift | Deleted file containing only commented-out code |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if c == 0 { return "0 votes" } | ||
| if c == 1 { return "1 vote" } | ||
| return "\(c) votes" | ||
| "\(viewModel.voteCount) vote\(viewModel.voteCount == 1 ? "" : "s")" |
There was a problem hiding this comment.
[nitpick] The new implementation no longer handles the '0 votes' case specially. While '0 votes' is grammatically correct, screen readers may benefit from the explicit handling. Consider if the previous distinct handling of '0 votes' was intentional for accessibility purposes.
| "\(viewModel.voteCount) vote\(viewModel.voteCount == 1 ? "" : "s")" | |
| viewModel.voteCount == 0 | |
| ? "No votes yet" | |
| : "\(viewModel.voteCount) vote\(viewModel.voteCount == 1 ? "" : "s")" |
There was a problem hiding this comment.
Not as per the previous behavior, but since this improves clarity for accessibility, I think it’s a good change to keep.
|
@AvdLee Please review |
🧩 Pull Request Summary
✨ Overview
This PR focuses on improving code efficiency, readability, and maintainability across multiple files by removing redundant logic, optimizing lookups, and simplifying structure.
🔍 Detailed Changes
🧠
RoadmapViewModel.swift⚡
RoadmapFeature.swiftguard— now safely defaults to an empty array as fallback.🗑️
ArrayExtensions.swift🎛️
RoadmapVoteButton.swift🎨
ColorExtensions.swift,RoadmapView.swift,RoadmapStyle.swift,RoadmapTemplates.swiftreturnstatements for improved consistency and readability.✅ Improvements
Type: Refactor / Cleanup / Performance
Impact: Internal (no functional changes to user-facing behavior)