未使用コードの削除: DevicePairingUsecase #9
Conversation
|
Warning Rate limit exceeded@harutiro has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 49 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Walkthrough未使用の Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (10)
76-78: 0.1秒の固定sleepは不要です。UserDefaults.set は同一プロセス内では同期です。この待機は削除可能です。
適用diff:
- // 設定されるまで少し待つ - try await Task.sleep(nanoseconds: 100_000_000) // 0.1秒
84-86: 固定sleepではなく条件待ちに置換してください（フレーク対策）。ロード完了条件に基づくポーリングにすることでテストの安定性が上がります。
適用diff:
- // UserDefaultsの変更通知とデータ読み込みが完了するまで待つ - try await Task.sleep(nanoseconds: 300_000_000) // 0.3秒 + // currentFloorMapInfo がセットされるまで最大2秒待つ（50msポーリング） + await waitUntil(timeout: 2.0) { await viewModel.currentFloorMapInfo != nil }ファイル外追加（ヘルパー）:
// テスト用ヘルパー private func waitUntil(timeout: TimeInterval = 2.0, pollInterval: UInt64 = 50_000_000, condition: @escaping () async -> Bool) async { let deadline = Date().addingTimeInterval(timeout) while Date() < deadline { if await condition() { return } try? await Task.sleep(nanoseconds: pollInterval) } }
155-156: 初期化直後の固定sleepは避け、必要性を再確認してください。副作用がなければ削除、必要なら明確な条件を待機する形へ。
288-294: ここも0.1秒sleepは不要です。UserDefaults への保存は同期。削除でテスト短縮可。
適用diff:
- // 設定されるまで少し待つ - try await Task.sleep(nanoseconds: 100_000_000) // 0.1秒
300-302: 固定sleep→条件待ちに統一してください。ロード完了（currentFloorMapInfo != nil）を待機。
適用diff:
- // UserDefaultsの変更通知とデータ読み込みが完了するまで待つ - try await Task.sleep(nanoseconds: 300_000_000) // 0.3秒 + await waitUntil(timeout: 2.0) { await viewModel.currentFloorMapInfo != nil }
354-356: 固定sleepは避けてください。状態安定化のための待機は条件ベースに。
適用diff:
- // 初期状態を安定させるため待機 - try await Task.sleep(nanoseconds: 300_000_000) // 0.3秒 + // 初期状態が nil のままであることを短時間確認（必要な場合） + await waitUntil(timeout: 0.5) { await viewModel.currentFloorMapInfo == nil }
374-374: 固定sleep→条件待ちに置換してください。UserDefaults変更の反映完了（VM側プロパティ更新）を条件に。
適用diff:
- try await Task.sleep(nanoseconds: 500_000_000) // 0.5秒 + await waitUntil(timeout: 2.0) { await viewModel.currentFloorMapInfo != nil }
379-381: ここも固定sleepを条件待ちへ。反映完了を確認してからアサーションへ進む形に。
適用diff:
- // 処理が完了するまで待つ - try await Task.sleep(nanoseconds: 500_000_000) // 0.5秒 + // 処理完了を確認 + await waitUntil(timeout: 2.0) { await viewModel.currentFloorMapInfo != nil }
389-394: printによるデバッグ出力は最小化を。テストログのノイズ源になります。必要なら失敗メッセージの情報量を上げる方針で十分です。
適用diff:
- // デバッグ情報を出力 - let debugData = UserDefaults.standard.data(forKey: "currentFloorMapInfo") - print("🔍 デバッグ: UserDefaultsのデータ存在: \(debugData != nil)") - if let data = debugData, let debugInfo = try? JSONDecoder().decode(FloorMapInfo.self, from: data) { - print("🔍 デバッグ: UserDefaultsから復元されたデータ: \(debugInfo.id)") - } + // （任意）追加の #expect メッセージで十分に診断可能です補足（任意改善）:
- setupTestEnvironment で Documents/FloorMaps を作成していますが、クリーンアップで削除していません。テスト専用の一時ディレクトリ（FileManager.default.temporaryDirectory 配下）に切り替えると副作用が減ります。
398-400: テスト終了時の synchronize も削除してください。併せてクリーンアップを強化。
- synchronize は不要。
- 併せて setup で作成した FloorMaps ディレクトリの削除も検討を。
適用diff:
- UserDefaults.standard.synchronize()ファイル外追加（任意・クリーンアップ強化例）:
private func cleanupTestEnvironment() { UserDefaults.standard.removeObject(forKey: "currentFloorMapInfo") // 追加: setup で作成したディレクトリの削除（存在する場合のみ） let fileManager = FileManager.default if let documentsPath = fileManager.urls(for: .documentDirectory, in: .userDomainMask).first { let floorMapsDir = documentsPath.appendingPathComponent("FloorMaps") try? fileManager.removeItem(at: floorMapsDir) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (1)
UWBViewerSystem/Presentation/Scenes/FloorMapTab/SystemCalibrationPage/SimpleCalibrationViewModel.swift (1)
loadInitialData(164-168)
🔇 Additional comments (3)
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift (3)
362-364: エンコード失敗時に明示的に throw するのは良いです。失敗時の原因特定がしやすくなっています。
366-372: 保存→読戻しの検証は妥当です。状態の確実性が上がっています。
377-377: 手動での再読み込みは妥当です。通知取りこぼしのフォールバックとして有効です。
| // 他のテストで設定されたUserDefaultsをすべてクリア | ||
| UserDefaults.standard.removeObject(forKey: "currentFloorMapInfo") | ||
| UserDefaults.standard.synchronize() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
UserDefaults.standard.synchronize() は非推奨・不要です。
削除してください。加えて、グローバルUserDefaults共有は並列実行時に競合します。可能ならテスト用スイート（suiteName）やDIで隔離を。
適用diff:
- UserDefaults.standard.synchronize()テスト内での synchronize 使用箇所の洗い出し:
🏁 Script executed:
#!/bin/bash
rg -nP 'UserDefaults\.standard\.synchronize\(\)'Length of output: 320
UserDefaults.standard.synchronize() を削除し、テスト用に UserDefaults を分離する
synchronize() は非推奨／不要です。以下の全出現箇所を削除し、並列テストで競合しないようテスト専用の UserDefaults(suiteName:) または DI による注入で隔離してください。
該当箇所: UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift — lines 349, 367, 399
🤖 Prompt for AI Agents
UWBViewerSystemTests/SimpleCalibrationViewModelTests.swift around lines 347–350,
367 and 399: remove all calls to UserDefaults.standard.synchronize() (they are
deprecated/unnecessary) and stop using UserDefaults.standard in tests; instead
isolate test storage by using a test-specific UserDefaults instance (e.g.,
UserDefaults(suiteName: "UWBViewerSystemTests") or accept a UserDefaults via DI
into the view model) and update each test to use that instance, ensuring cleanup
between tests by removing the suite's persistent domain (or removing keys from
that suite) so parallel tests won't conflict.
- UWBViewerSystem/Domain/Usecase/DevicePairingUsecase.swift (349行) を削除 - プロジェクト内で一切参照されていない完全なデッドコードでした - 削除後もビルドエラーは発生せず、プロジェクトは正常に動作します - コードベースの削減とメンテナンス対象の削減により可読性が向上しました fixes #6 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
harutiro
force-pushed
the
issue-6-remove-device-pairing-usecase
branch
2 times, most recently
from
7b130e1 to
3bfc45d
Compare
修正完了のご報告@coderabbitai レビューコメントありがとうございます。 指摘された問題の修正状況1. 範囲外変更について:
2. テスト成功について:
修正内容の詳細削除したファイル:
付随する修正:
検証結果
Issue #6の完了条件を満たし、すべてのテストが成功している状態です。 ご確認のほど、よろしくお願いいたします。 |
|
Tip For best results, initiate chat on the files or code changes. @harutiro ありがとうございます！修正完了の詳細なご報告をいただき、とても分かりやすく説明していただけました。 修正内容の確認: 特に良かった点:
Issue #6の目標達成:
完璧な作業でした。デッドコードの削除により、プロジェクトがより整理され、将来のメンテナンスも楽になりますね！ 🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
概要
完全に未使用のUsecaseクラス
DevicePairingUsecase.swiftを削除しました。変更内容
UWBViewerSystem/Domain/Usecase/DevicePairingUsecase.swift(349行) を削除削除の根拠
テスト結果
影響
メリット
fixes #6
🤖 Generated with Claude Code
Summary by CodeRabbit