diff --git a/references/smells.md b/references/smells.md new file mode 100644 index 0000000..5d8abfc --- /dev/null +++ b/references/smells.md @@ -0,0 +1,158 @@ +# 程式碼壞味道審查清單 + +來源:《Refactoring》(Martin Fowler)與《A Philosophy of Software Design》(John Ousterhout,第 6 組)。 +`code-review` 技能依此清單分六組審查,每組由一個 sub agent 執行。 +每項格式:**定義 / 偵測訊號 / 建議重構手法**。 + +## 第 1 組:結構與體積問題(Bloaters) + +### 1.1 巨型類別(Large Class) + +- **定義**:單一 Class 數千行,包山包海,違反單一職責原則。 +- **偵測訊號**:類別行數過大(> 500 行需注意、> 1000 行必列);欄位過多;方法分成多個互不相干的群組;類別名稱含 Manager、Util、Helper 等萬用字。 +- **建議重構手法**:Extract Class、Extract Subclass、Extract Interface;依職責拆分後以組合取代繼承。 + +### 1.2 臃腫函式(Long Method) + +- **定義**:一個 Function 寫了幾百行,邏輯複雜到沒人敢動。 +- **偵測訊號**:函式行數 > 50 行需注意、> 100 行必列;一個函式內有多段以空行或註解分隔的「段落」;區域變數過多;圈複雜度高。 +- **建議重構手法**:Extract Function(每個段落抽成具名函式)、Replace Temp with Query、Decompose Conditional、Replace Function with Command。 + +### 1.3 過長參數列(Long Parameter List) + +- **定義**:傳入超過 4、5 個以上的參數,呼叫時極易傳錯。 +- **偵測訊號**:參數 ≥ 5 個必列、4 個需注意;連續同型別參數(例:三個 string 相鄰);boolean 旗標參數;呼叫端常以固定組合傳值。 +- **建議重構手法**:Introduce Parameter Object、Preserve Whole Object、Replace Parameter with Query、Remove Flag Argument。 + +### 1.4 基本型態偏執(Primitive Obsession) + +- **定義**:不用物件包裝,全部用字串或數字代替(金額、電話、狀態碼、範圍…)。 +- **偵測訊號**:以 string/int 表達有格式或規則的概念;同一組基本型態欄位在多處一起出現;以字串常數當型別碼;到處重複的驗證邏輯。 +- **建議重構手法**:Replace Primitive with Object、Replace Type Code with Subclasses、Introduce Parameter Object、Extract Class。 + +## 第 2 組:可讀性與命名問題(Obscurity) + +### 2.1 神秘命名(Mysterious Name) + +- **定義**:使用毫無意義的變數名(a、tmp、data2、doStuff)。 +- **偵測訊號**:單字母或縮寫命名(迴圈索引除外);名稱與實際行為不符;需要讀實作才知道用途;名稱含編號(data1、data2)。 +- **建議重構手法**:Rename Variable、Rename Function、Rename Field;名稱表達「意圖」而非「實作」。 + +### 2.2 魔術數字(Magic Numbers) + +- **定義**:程式碼中直接出現沒人知道代表什麼的字面值。 +- **偵測訊號**:條件式或運算中出現裸數字/裸字串(0、1、-1、空字串與明顯單位換算除外需判斷);同一字面值在多處出現。 +- **建議重構手法**:Replace Magic Literal(抽成具名常數或 enum);有行為就升級為 Replace Type Code with Class。 + +### 2.3 死碼(Dead Code) + +- **定義**:已經不用卻不刪除的註解掉程式碼、變數、函式或類別。 +- **偵測訊號**:被註解掉的程式碼區塊;無人呼叫的函式/類別(grep 全庫零引用);永遠不成立的條件分支;未使用的 import、參數、變數。 +- **建議重構手法**:Remove Dead Code(直接刪除,歷史交給版本控制)。 + +### 2.4 過度註解(Comments) + +- **定義**:因為程式碼寫太爛,只好寫一堆註解來解釋邏輯;好的程式碼應該能「自我解釋」。 +- **偵測訊號**:註解在解釋「這段在做什麼」而非「為什麼這樣做」;註解與程式碼不同步;每隔幾行就一條註解;用註解分段的長函式(同時列 1.2)。 +- **建議重構手法**:Extract Function(用函式名取代註解)、Rename(用命名取代註解)、Introduce Assertion;保留「為什麼」與外部限制類註解。 +- **注意**:本項與「註解問題」(第 5 組)互補:刪除解釋性廢話註解,補齊第 5 組要求的介面契約註解,兩者不衝突。 + +## 第 3 組:耦合與設計問題(Couplers) + +### 3.1 依賴嫉妒(Feature Envy) + +- **定義**:A 類別的函式不斷去存取、修改 B 類別的資料,顯示該函式放錯地方。 +- **偵測訊號**:函式內對別的物件的 getter/setter 呼叫次數多於對自身成員的使用;連續鏈式取值後運算(`order.customer.address.city`)。 +- **建議重構手法**:Move Function(搬到資料所在的類別)、Extract Function 後再 Move。 + +### 3.2 散彈式修改(Shotgun Surgery) + +- **定義**:每當要修改一個小功能,就必須同時修改多個不同的檔案。 +- **偵測訊號**:本次 diff 為了單一需求橫跨多檔做同質小改動;同一常數/規則散落多處;歷史上同組檔案總是一起被改。 +- **建議重構手法**:Move Function / Move Field 集中職責、Combine Functions into Class、Inline Function 後重新抽取。 + +### 3.3 發散式變化(Divergent Change) + +- **定義**:一個類別因為各種完全不相關的原因需要被修改。 +- **偵測訊號**:類別的修改歷史來自多種不相干需求;類別內方法可依「變更原因」分成多群;「如果要加 X 就改這裡,要加 Y 也改這裡」。 +- **建議重構手法**:Split Phase、Extract Class、Move Function,讓每個模組只有一個變更理由。 + +### 3.4 親密關係(Inappropriate Intimacy) + +- **定義**:兩個類別過度了解彼此的私有實作細節。 +- **偵測訊號**:存取對方的 private/internal 成員或繞過封裝(反射、friend、直接操作內部集合);雙向依賴;子類別依賴父類別實作細節。 +- **建議重構手法**:Move Function / Move Field、Change Bidirectional to Unidirectional、Hide Delegate、Replace Subclass with Delegate。 + +## 第 4 組:邏輯與壞習慣(Dispensables & Others) + +### 4.1 重複程式碼(Duplicated Code) + +- **定義**:到處複製貼上(Copy-Paste),改一個 Bug 要改好幾個地方。 +- **偵測訊號**:相同或僅參數不同的程式片段出現 ≥ 2 處;兄弟類別有相同方法;diff 中新增的程式碼與既有程式碼雷同。 +- **建議重構手法**:Extract Function、Pull Up Method、Form Template Method、Slide Statements 後合併。 + +### 4.2 巢狀地獄(Nested If Hell) + +- **定義**:If-Else 或 Loop 疊了 5、6 層以上,箭頭型程式碼(Arrow Anti-pattern)。 +- **偵測訊號**:縮排深度 ≥ 4 層需注意、≥ 5 層必列;else 鏈過長;條件式中混合多個否定。 +- **建議重構手法**:Replace Nested Conditional with Guard Clauses(衛語句早退)、Decompose Conditional、Replace Conditional with Polymorphism、Extract Function。 + +### 4.3 誇誇其談未來性(Speculative Generality) + +- **定義**:為了解決「未來可能」會用到的功能,寫了一堆現在用不到的複雜架構。 +- **偵測訊號**:只有一個實作的抽象層/介面;從未被覆寫的 hook 方法;只在測試中使用的參數或彈性;「以後可能會需要」的註解。 +- **建議重構手法**:Collapse Hierarchy、Inline Function / Inline Class、Remove Dead Code、移除未用參數(Change Function Declaration)。 + +### 4.4 吞掉異常(Swallowed Exceptions) + +- **定義**:try-catch 裡面留白,發生錯誤時直接隱瞞,導致難以 Debug。 +- **偵測訊號**:空的 catch 區塊;catch 後只留註解或 `// ignore`;catch 住廣義 Exception 後回傳 null/預設值而不記錄;錯誤訊息被丟棄後重包。 +- **建議重構手法**:最少要記錄(log)並保留原始例外鏈;能處理才 catch,不能處理就往上拋;以 Introduce Special Case 取代以 null 掩蓋錯誤。 + +## 第 5 組:註解問題(介面契約註解) + +此組檢查「該有而沒有」的註解,與 2.4(該刪的註解)互補。 + +### 5.1 方法描述 + +- **定義**:每個公開方法要有一句話描述,並標明所屬層級:**顯示層 / 邏輯層 / 存取層**。 +- **偵測訊號**:公開方法無描述;描述未標層級;描述與方法實際行為不符。 +- **建議重構手法**:補上單句描述 + 層級標記;若一個方法橫跨多層,先依 Split Phase 拆分再各自標註。 + +### 5.2 輸入參數說明 + +- **定義**:所有輸入參數都要有用途說明。 +- **偵測訊號**:參數無說明;說明只是重複參數名稱;可選參數未說明預設行為。 +- **建議重構手法**:逐參數補「用途」說明;參數多到說明困難時同時列 1.3 並建議 Introduce Parameter Object。 + +### 5.3 輸出說明 + +- **定義**:輸出(回傳值)要說明回傳的資料內容概要。 +- **偵測訊號**:無回傳說明;未說明 null/空集合/錯誤時的回傳;回傳布林但未說明 true/false 意義。 +- **建議重構手法**:補回傳內容概要與邊界情況說明。 + +### 5.4 輸入與輸出範例 + +- **定義**:輸入與輸出參數都必須有範例;範例內容**優先嘗試從資料庫取得真實資料,失敗才透過邏輯推理**產生。 +- **偵測訊號**:註解缺範例;範例與型別不符;範例顯然是佔位假資料而環境可取得真實資料。 +- **建議重構手法**:以可用的連線查詢一筆代表性資料當範例(去識別化,不可含個資);無法連線才以邏輯推理造出合理範例並標明為推理值。 + +### 5.5 巢狀結構註解 + +- **定義**:如果參數有巢狀結構(例如 class 內還有 class),就必須完全補齊每一層的註解。 +- **偵測訊號**:DTO/ViewModel 僅頂層有註解;內層類別、集合元素型別的欄位無說明或無範例。 +- **建議重構手法**:逐層補齊 5.1–5.4;巢狀過深(≥ 3 層)時同時評估 Extract Class 是否被濫用。 + +## 第 6 組:淺模組(Shallow Module) + +- **定義**:介面複雜度相對於功能深度過高的模組——使用它要懂的事,跟自己寫差不多(出自《A Philosophy of Software Design》:好模組要「介面簡單、實作深」)。 +- **偵測訊號**:只有一行轉呼叫(pass-through)的方法或類別;包裝層與被包裝者介面幾乎相同;參數原封不動往下傳的層層委派;為每個底層方法都開一個對應方法的「殼」。 +- **建議重構手法**:Inline Class / Inline Function 移除殼層;或反向加深模組——把散在呼叫端的邏輯(驗證、轉換、錯誤處理)收進模組內,讓介面吸收複雜度。 + +## 嚴重度分級 + +| 級別 | 意義 | 例 | +| --- | --- | --- | +| 高 | 會造成錯誤或已阻礙修改 | 吞掉異常、重複程式碼改漏、死碼誤導 | +| 中 | 持續增加維護成本 | 巨型類別、臃腫函式、巢狀地獄、Couplers 全組 | +| 低 | 可讀性與一致性 | 命名、魔術數字、註解缺漏、淺模組 | diff --git a/skills/code-review/SKILL.md b/skills/code-review/SKILL.md new file mode 100644 index 0000000..8962083 --- /dev/null +++ b/skills/code-review/SKILL.md @@ -0,0 +1,41 @@ +--- +name: code-review +description: Review changed code against the Refactoring smell catalog in six groups (bloaters, obscurity, couplers, dispensables, comment contract, shallow modules). Run when a file change is complete or an implementation is complete, e.g. from jsc-sdlc implement. Each group runs as a sub agent over the git diff; findings are reported with file:line, severity, and refactoring, and the caller decides whether to fix. Not a replacement for the CLI's built-in security or bug review. +--- + +# code-review — 壞味道分組審查 + +依 `references/smells.md`(來自《Refactoring》)審查變更的程式碼。 + +## 審查時機 + +1. **檔案變更完成時**:單檔或一組相關檔案改完。 +2. **實作完成時**:一個工作包的所有待辦完成(`jsc-sdlc:implement` 步驟 6 呼叫)。 + +## 分工 + +- 本技能專注《Refactoring》壞味道與註解契約(第 5 組)與淺模組(第 6 組)。 +- 安全性、邏輯 bug、測試涵蓋率交給 CLI 內建的 review 能力(例:claude 的 `/security-review`),不重複實作。 + +## 流程 + +1. 取得審查範圍:`git diff`(未 commit 變更)或 `git diff {base}...HEAD`(實作完成時對基準分支);列出變更檔案清單。 +2. 六組檢查分組進行,**每組必須以 sub agent 執行**,六組可平行: + + | 組 | 範圍 | + | --- | --- | + | 1 結構與體積(Bloaters) | smells.md 第 1 組 | + | 2 可讀性與命名(Obscurity) | smells.md 第 2 組 | + | 3 耦合與設計(Couplers) | smells.md 第 3 組 | + | 4 邏輯與壞習慣(Dispensables & Others) | smells.md 第 4 組 | + | 5 註解問題(介面契約) | smells.md 第 5 組 | + | 6 淺模組(Shallow Module) | smells.md 第 6 組 | + + 每個 sub agent 的指示:只讀不改;依該組的「定義 / 偵測訊號」逐檔檢查變更行與其所在函式/類別;每筆發現回報 `檔案:行號`、壞味道名稱、嚴重度(高/中/低,依 smells.md 分級)、一句話證據、建議重構手法。 +3. 彙整六組發現:去除重複(同位置多組命中時合併並列出所有壞味道)、依嚴重度排序。 +4. 回報審查結果清單。**本技能不修改程式碼**;是否修正由呼叫端決定(實作流程中通常高、中必修,低擇要修)。 + +## 注意 + +- 第 5 組範例資料若查詢資料庫取得,必須去識別化,不可含個資。 +- 無任何發現時明確回報「無發現」,不可留白。