unspaghettify-your-code

エージェントスキル: 成熟したコードを3層アーキテクチャにリファクタリングする

哲学: いつリファクタリングすべきか

❌ リファクタリングすべきでないとき

• •新機能の実装中: まず動かすことが優先
• •要件が不明確: 何が正解か分からない段階で構造化は無意味
• •プロトタイピング/PoC段階: 検証が目的、コード品質は二の次
• •1回限りのスクリプト: 使い捨てコードに投資しない

原則: "Premature optimization is the root of all evil" (早すぎる最適化は諸悪の根源)

✅ リファクタリングすべきとき

以下のすべてを満たす場合:

1. •要件が固まった: 何を作るべきか明確になった
2. •動作検証済み: PoC/プロトタイプで動作を確認した
3. •パターンが見えた: 繰り返しや責務の混在が明確になった
4. •保守が必要: これから長期的にメンテナンスする
5. •拡張予定: 類似機能を追加する計画がある

原則: "Make it work, make it right, make it fast"

リファクタリング判断基準

定量的な基準

定性的な基準

• •"ここに何を書けばいいか分からない" → 責務が不明確
• •"変更の影響範囲が予測できない" → 結合度が高い
• •"テストが書けない" → 依存関係が複雑
• •"似たコードを3回書いた" → 抽象化の機会

3層アーキテクチャ方針

目的

• •関心の分離: ハードウェア/ビジネスロジック/UIの分離
• •テスタビリティ: 各層を独立してテスト可能
• •再利用性: HAL層は他プロジェクトでも利用可能
• •保守性: 変更の影響範囲を限定

層の定義

┌─────────────────────────────────────────┐
│  Layer 3: Application Layer             │
│  - main(), CLI parsing, signal handling │
│  - 責務: ライフサイクル管理               │
│  - サイズ: ~100行                        │
└─────────────────────────────────────────┘
                    ↓
┌─────────────────────────────────────────┐
│  Layer 2: Pipeline/Business Logic       │
│  - ワークフロー統合、データフロー         │
│  - 責務: 複数のHALを組み合わせた処理      │
│  - サイズ: ~200-300行/ファイル            │
└─────────────────────────────────────────┘
                    ↓
┌─────────────────────────────────────────┐
│  Layer 1: Hardware Abstraction (HAL)    │
│  - ハードウェアAPI の薄いラッパー         │
│  - 責務: 単一ハードウェアの操作           │
│  - サイズ: ~200-300行/ファイル            │
└─────────────────────────────────────────┘

各層の責務

Layer 1: HAL (Hardware Abstraction Layer)

DO (推奨):

• •✅ ハードウェアAPIの直接呼び出しをカプセル化
• •✅ エラーハンドリングを統一
• •✅ リソース管理 (create/destroy) を提供
• •✅ ハードウェア固有の設定を隠蔽

DON'T (禁止):

• •❌ ビジネスロジックを含めない
• •❌ 他のHAL層を呼ばない (依存しない)
• •❌ グローバル状態を持たない
• •❌ アプリケーション固有の処理を含めない

サンプル (今回の実装):

// vio_lowlevel.h - VIO HAL
int vio_create(vio_context_t *ctx, int camera_index, ...);
int vio_start(vio_context_t *ctx);
int vio_get_frame(vio_context_t *ctx, hbn_vnode_image_t *frame, int timeout_ms);
void vio_destroy(vio_context_t *ctx);

Layer 2: Pipeline/Business Logic

DO (推奨):

• •✅ 複数のHAL層を組み合わせる
• •✅ データフローを制御
• •✅ ビジネスロジックを実装
• •✅ 状態管理

DON'T (禁止):

• •❌ ハードウェアAPIを直接呼ばない
• •❌ main()を含めない
• •❌ CLI引数解析を含めない

サンプル (今回の実装):

// camera_pipeline.h - VIO + Encoder統合
typedef struct {
    vio_context_t vio;           // HAL層を利用
    encoder_context_t encoder;   // HAL層を利用
    SharedFrameBuffer *shm_h264;
    volatile bool *running_flag;
} camera_pipeline_t;

int pipeline_run(camera_pipeline_t *pipeline, volatile bool *running_flag);

Layer 3: Application Layer

DO (推奨):

• •✅ main()関数
• •✅ CLI引数解析
• •✅ シグナルハンドリング
• •✅ ライフサイクル管理

DON'T (禁止):

• •❌ ビジネスロジックを含めない
• •❌ ハードウェアAPIを直接呼ばない
• •❌ 複雑な処理を含めない (Pipeline層に委譲)

サンプル (今回の実装):

// camera_daemon_main.c
int main(int argc, char *argv[]) {
    // 1. Parse arguments
    // 2. Signal handling
    // 3. Create pipeline (Pipeline層を利用)
    // 4. Start pipeline
    // 5. Run pipeline
    // 6. Cleanup
}

命名規則

ファイル命名

関数命名

インフラストラクチャ

ログライブラリ

要件:

• •ログレベル (DEBUG, INFO, WARN, ERROR)
• •モジュール名表示
• •スレッドセーフ
• •組み込みシステムで軽量 (~150行)

実装: logger.c/h

使い方:

#include "logger.h"

// 初期化 (main関数で1回)
log_init(LOG_LEVEL_INFO, stdout, 0);

// 各モジュールで使用
LOG_INFO("VIO", "Pipeline created successfully");
LOG_ERROR("Encoder", "hb_mm_mc_configure failed: %d", ret);

推奨:

• •✅ printf/fprintf の代わりにLOG_*マクロを使う
• •✅ モジュール名は固定文字列 (ファイル名由来)
• •✅ エラー時は必ずエラーコードも出力

禁止:

• •❌ printfでエラーを出力しない (LOG_ERRORを使う)
• •❌ ログレベルを無視しない (DEBUG情報はLOG_DEBUGで)

リファクタリングプロセス

Phase 1: 分析

1. •

   既存コードの理解

   • •責務を列挙 (ハードウェア操作 / ビジネスロジック / UI)
   • •依存関係を図示
   • •重複コードを特定
2. •

   境界の特定

   • •どこでHAL層とPipeline層を分けるか
   • •どのハードウェアをカプセル化するか

Phase 2: HAL層の抽出

1. •

   1つのハードウェアに集中

   • •例: VIO (VIN→ISP→VSE)
   • •例: Encoder (H.264)
2. •

   最小限のAPIを設計

   • •create/start/process/stop/destroy パターン
   • •context構造体でステートを管理
3. •

   既存コードから抽出

   • •ハードウェアAPIの呼び出しをコピー
   • •エラーハンドリングを統一
   • •ログ出力を統一
4. •

   テスト

   • •元のコードと同じ動作をするか確認

Phase 3: Pipeline層の抽出

1. •

   ワークフローを特定

   • •例: VIO → Encoder → 共有メモリ
2. •

   HAL層を組み合わせる

   • •複数のHAL contextを保持
   • •データフローを実装
3. •

   テスト

   • •エンドツーエンドで動作確認

Phase 4: Application層の簡素化

1. •

   main()をシンプルに

   • •引数解析
   • •Pipeline層の呼び出し
   • •クリーンアップ
2. •

   テスト

   • •元の機能と同等か確認
   • •パフォーマンス検証 (FPS等)

プロジェクト固有の詳細

D-Robotics X5 固有

ハードウェア制約

重要API

VIO (Low-level):

• •hbn_camera_create() - カメラ初期化
• •hbn_vnode_open() - VIN/ISP/VSEノード作成
• •hbn_vflow_bind_vnode() - パイプライン接続
• •hbn_vnode_getframe() - フレーム取得

Encoder (Low-level):

• •hb_mm_mc_initialize() - エンコーダー初期化
• •hb_mm_mc_configure() - エンコーダー設定
• •hb_mm_mc_start() - エンコーダー起動
• •hb_mm_mc_queue_input_buffer() - NV12入力
• •hb_mm_mc_dequeue_output_buffer() - H.264出力

ビルド設定

必須ライブラリ:

LDLIBS := -lrt -lpthread -lcam -lvpf -lhbmem -lmultimedia

ヘッダーインクルード:

#include "hb_camera_interface.h"
#include "hb_camera_data_config.h"
#include "hbn_api.h"
#include "hb_media_codec.h"
#include "hb_mem_mgr.h"

成功指標

リファクタリングが成功したかの判断基準:

必須条件

• •✅ 機能等価性: 元のコードと同じ動作をする
• •✅ 性能維持: FPS等のパフォーマンスが劣化しない
• •✅ ビルド成功: 警告なしでビルドできる

推奨条件

• •✅ ファイルサイズ: 各ファイルが300行以下
• •✅ 責務明確: 各ファイルの役割が1文で説明できる
• •✅ 依存最小: 各層が下の層のみに依存
• •✅ テスト可能: 各層を独立してテスト可能
• •✅ ログ整理: 統一されたログ出力

チェックリスト

リファクタリング前

• • 既存コードが動作している
• • 要件が固まっている
• • パフォーマンス目標を達成している
• • 重複/責務混在が明確になっている

リファクタリング中

• • HAL層が完成し、単体で動作する
• • Pipeline層が完成し、HAL層を使って動作する
• • Application層がシンプルになった
• • ログライブラリを全体に統合した

リファクタリング後

• • 元のコードと同じ動作をする
• • パフォーマンスが維持されている
• • 警告なしでビルドできる
• • ドキュメントを更新した (architecture_refactor_plan.md等)
• • 制約事項を記録した (ビットレート制限等)

参考文献

• •Martin Fowler - "Refactoring: Improving the Design of Existing Code"
• •Robert C. Martin - "Clean Architecture"
• •Kent Beck - "Test Driven Development: By Example"

このスキルの使い方

ユーザーがリファクタリングを依頼する場合

User: "camera_daemon_drobotics.c (886行) をリファクタリングしたい"

Agent:
1. まず判断基準を確認します
   - 要件は固まっていますか？
   - 動作検証は済んでいますか？
   - 保守・拡張の予定はありますか？

2. (条件を満たす場合) Phase 1: 分析
   - ファイルを読み、責務を列挙
   - HAL層候補を特定

3. Phase 2-4: 実装
   - HAL層を抽出
   - Pipeline層を作成
   - Application層を簡素化

4. 検証
   - パフォーマンステスト
   - 動作確認

エージェントが自動判断する場合

エージェントは以下の状況で自動的にリファクタリングを提案します:

• •ファイルが500行を超え、複数の責務が混在している
• •ユーザーが「整理したい」「構造化したい」と発言した
• •類似コードが3箇所以上に出現した

提案形式:

"このファイルは{行数}行で、{責務1}と{責務2}が混在しています。
3層アーキテクチャにリファクタリングすることで:
- 各ファイルが{予想行数}行程度に収まる
- テスタビリティが向上する
- 保守性が向上する

リファクタリングを実施しますか？"

アンチパターン

❌ 早すぎるリファクタリング

NG例:

// たった50行の単純なスクリプトを3層に分割
// → 過度な複雑化、メンテナンスコストが上がるだけ

正解: 小さくシンプルなコードはそのまま残す

❌ 過度な抽象化

NG例:

// 1箇所でしか使わない処理をわざわざ関数化
AbstractFactoryBuilder* builder = createAbstractFactoryBuilder();
// → YAGNI違反

正解: 3箇所以上で重複してから抽象化を検討

❌ 無意味な分割

NG例:

// getter/setterだけのファイル
// utils.c に何でも詰め込む
// → 責務が不明確

正解: 明確な責務を持つ単位で分割

今回のケーススタディ

Before (PoC段階)

• •camera_poc_lowlevel.c - 639行
• •VIO操作 + Encoder操作 + メインループが混在
• •目的: 30fps達成の検証 → ✅ 成功

判断

• •要件固定: 2カメラ同時30fps
• •動作検証済み: PoCで達成
• •保守予定: デーモンとして長期運用
• •拡張予定: デコーダー追加など

→ リファクタリング決定

After (リファクタリング後)

• •vio_lowlevel.c/h - 334行 (HAL)
• •encoder_lowlevel.c/h - 203行 (HAL)
• •camera_pipeline.c/h - 217行 (Pipeline)
• •camera_daemon_main.c - 177行 (Application)
• •logger.c/h - 106行 (Infrastructure)

合計: 1037行 (PoC 639行から400行増)

トレードオフ:

• •コード量は増えたが、各ファイルの責務が明確に
• •テスタビリティ向上 (各層を独立テスト可能)
• •再利用性向上 (HAL層は他プロジェクトでも使用可)
• •保守性向上 (変更の影響範囲が限定)

結果:

• •機能等価性: ✅ 同じ動作
• •性能維持: ✅ Camera 0: 29.87 FPS, Camera 1: 30.40 FPS
• •ビルド: ✅ 警告なし

まとめ

リファクタリングの金言:

"Make it work, make it right, make it fast"

まず動かせ。次に正しくしろ。最後に速くしろ。

リファクタリングすべきタイミング:

要件が固まり、動作が確認でき、長期保守が必要になったとき。 それまでは、汚くても素早く回せ。