refactor(#23): SpectrumView を struct/geometry/rendering に分割#305
Merged
Conversation
View ファイルは純粋な SwiftUI struct のみを残し、bar ジオメトリを SpectrumGeometry.swift（純粋・テスト可能）、Canvas 描画を SpectrumRendering.swift（GraphicsContext 依存・薄い）へ自由関数として分離。 body はそれらを呼ぶだけで計算・描画メソッドを持たない。
分離した barsPath 自由関数を直接テスト（空/単一/複数 bar の union 境界）。
XxxxView.swift は純粋 struct のみ・ロジックは Geometry/Rendering 兄弟ファイルへ、を明文化。
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR extracts Spectrum view geometry and Canvas drawing into ChangesSpectrum view refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SpectrumView
participant SpectrumGeometry
participant SpectrumRenderer
SpectrumView->>SpectrumGeometry: stripDepth / alignment / trackExtent(placement, style)
SpectrumGeometry-->>SpectrumView: layout metrics
SpectrumView->>SpectrumRenderer: draw(context, size, heights, style)
SpectrumRenderer->>SpectrumGeometry: barRects(size, heights, placement)
SpectrumGeometry-->>SpectrumRenderer: [SpectrumBar]
SpectrumRenderer-->>SpectrumView: filled GraphicsContext
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
SpectrumView.swift を「宣言だけの SwiftUI View」に保ちつつ、スペクトラム描画のジオメトリ計算とCanvas 描画をそれぞれ別ファイルへ分離して、テスタビリティと見通しを改善するリファクタです。既存挙動は維持しつつ、barsPath のユニットテストを追加しています。
Changes:
SpectrumView.swiftからジオメトリ計算・描画ロジックを除去し、drawSpectrumBars呼び出しに集約- 純粋関数のジオメトリ群を
SpectrumGeometry.swiftに、Canvas 描画をSpectrumRendering.swiftに分離 barsPathの幾何テスト追加 + patch バージョン更新（2.20.0 → 2.20.1）
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/ViewsTests/SpectrumBarRectsTests.swift | barsPath の境界（空/単一/複数）を直接テストしてジオメトリの保証を強化 |
| Sources/Views/Spectrum/SpectrumView.swift | View を宣言中心に整理し、描画を drawSpectrumBars に委譲 |
| Sources/Views/Spectrum/SpectrumRendering.swift | GraphicsContext に依存する Canvas 描画ロジックを集約 |
| Sources/Views/Spectrum/SpectrumGeometry.swift | 描画に必要な rect/point/path/alignment 生成を純粋関数として切り出し |
| Sources/VersionHandler/Resources/version.txt | patch バンプ（2.20.1） |
| .claude/rules/swift-idioms.md | View/Geometry/Rendering のファイル分割方針をドキュメント化 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+52
to
+56
| **A `XxxxView.swift` file must contain only the pure SwiftUI `View` struct.** | ||
| Everything else — bar/rect geometry, gradient math, alignment helpers, and even | ||
| `GraphicsContext` drawing — is *logic*, not view declaration, so it lives in | ||
| sibling files. The `View` struct's `body` calls out to those free functions; it | ||
| never defines them or the drawing methods. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
グローバル空間の自由関数ではなく cohesive な struct にまとめ、Swift の OO 基盤に沿わせる。SpectrumGeometry は純粋メソッドの struct、SpectrumRenderer は @mainactor struct で resolver を @dependency で保持。SpectrumView は両者の インスタンスを持ち body から委譲。型名が名前空間を与えるのでメソッド名から spectrum 接頭辞を落とす（geometry.barRects 等）。テストはインスタンス経由に更新。
自由関数の袋ではなく struct にまとめ View がインスタンスを持つ方式、および これは Presenter ロジックではなく View 層の値→ピクセル写像である旨を明記。
codecov が指摘した未カバー箇所への対応。NSHostingView + ImageRenderer で 実際に Canvas を走らせ、SpectrumRenderer.draw/fill の全分岐（solid 塗り・ level/frequency/amplitude グラデーション・background 塗り・無音時の no-bars ガード）を通す。既存の HeaderView/RippleView テストと同じ render() パターンを踏襲。 結果: SpectrumRenderer/SpectrumGeometry/SpectrumView の3ファイルとも region/function/line カバレッジ 100% を達成。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Tests/ViewsTests/SpectrumViewRenderingTests.swift (2)
43-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
letformagnitudesValue.
magnitudesValueis only ever assigned ininitand never mutated elsewhere in this file. As per coding guidelines, "Everyvarshould have a real reason to exist."♻️ Proposed fix
- var magnitudesValue: [Float] + let magnitudesValue: [Float]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/ViewsTests/SpectrumViewRenderingTests.swift` around lines 43 - 48, `magnitudesValue` in `SpectrumViewRenderingTests` is never mutated after initialization, so replace the stored `var` with a `let` and keep the assignment only in the `init(style:magnitudes:)` initializer. Update the `SpectrumViewRenderingTests` fixture to use an immutable property while preserving the existing `spectrumStyle` and `magnitudesValue` setup.Source: Coding guidelines
18-26: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRemove the unused
NSHostingViewsetup
ImageRenderersnapshotsview.frame(...)directly, so theNSHostingViewlayout work here has no effect. Drop it, or update the comment if this helper is meant to rely on some other side effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/ViewsTests/SpectrumViewRenderingTests.swift` around lines 18 - 26, The unused NSHostingView setup in renderHosting is dead code because ImageRenderer already snapshots the SwiftUI view directly. Remove the hostingView creation, frame assignment, and layoutSubtreeIfNeeded call from renderHosting, and keep the helper centered on the ImageRenderer path unless you intend to document a different side effect in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Tests/ViewsTests/SpectrumViewRenderingTests.swift`:
- Around line 43-48: `magnitudesValue` in `SpectrumViewRenderingTests` is never
mutated after initialization, so replace the stored `var` with a `let` and keep
the assignment only in the `init(style:magnitudes:)` initializer. Update the
`SpectrumViewRenderingTests` fixture to use an immutable property while
preserving the existing `spectrumStyle` and `magnitudesValue` setup.
- Around line 18-26: The unused NSHostingView setup in renderHosting is dead
code because ImageRenderer already snapshots the SwiftUI view directly. Remove
the hostingView creation, frame assignment, and layoutSubtreeIfNeeded call from
renderHosting, and keep the helper centered on the ImageRenderer path unless you
intend to document a different side effect in the comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a30dc3b2-0fb0-4a54-a101-8a91e853abca
📒 Files selected for processing (1)
Tests/ViewsTests/SpectrumViewRenderingTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
SpectrumView.swiftを 純粋な SwiftUIViewstruct のみに削ぎ落とし、ジオメトリ計算と Canvas 描画をロジックとして分離する純粋なファイル再編。挙動は不変、テスト網羅を1つ追加。変更内容
SpectrumView.swift—struct SpectrumView: View（body）のみを残す。drawBars/fillBarsメソッドと全自由関数を撤去し、bodyは分離先を呼ぶだけに。SpectrumGeometry.swift（新規）—size/style→ rect・point・alignment・Pathの純粋関数群（spectrumBarRects・barStripDepth・gradientEnds・barsPathほか）。GraphicsContext非依存でユニットテスト可能。SpectrumRendering.swift（新規）— 還元不能な Canvas 描画（drawSpectrumBars/fillSpectrumBars）を自由関数化。実 context 無しでは動かないので設計上テスト対象外だが、描く値は全て tested な geometry 由来なので薄い。barsPathのジオメトリテストを追加（空/単一/複数 bar の union 境界）。従来は透過的にしか通っていなかった経路を直接テスト。.claude/rules/swift-idioms.mdに View ファイル構成ルール（XxxxView.swiftは純粋 struct のみ・ロジックは Geometry/Rendering 兄弟ファイルへ）を明文化。背景・動機
計算を抱えた View ファイルは、テスト不能な宣言部の周りに tested なロジックを引きずり、テスタビリティを下げる。ロジックを自由関数として外へ出せばテスト可能になり（"Untestable = 抽象化不足" 原則）、struct は何を描画するか一目で読める。テスタビリティでファイルを分けることで、カバレッジ境界がそのままファイル境界になる。
テスト計画
swift build成功swift test --filter ViewsTests全101テスト green（新規barsPathスイート含む）make format適用済み備考
Summary by CodeRabbit