程式開發至此，以經驗來說，功能複雜度已經算頗高，而開發至今，都還沒有做什麼整理，任憑 AI 幫我產出程式碼而已。是時候該整理重構一下，整裝再出發。
重構有個簡單卻容易犯的原則：
重構不能加上新功能邏輯
雖然知道，但我以前常犯（現在偶爾還是會），直到常常被耳提面命提醒，踩過幾次坑之後，才終於有所體悟。
加上功能之後，最直接的就是可掌控的邏輯複雜度飆速上升。原本只要管 1 個變化的地方，將這個功能在不變的前提下做重構，那如果出錯了，也只要找這邊就好。
如果自以為對需求很了解，不只調整舊有的功能，甚至還貪心多了新增的功能。幸運的話，可以很順利地重構並新增該功能；然而大多數時候，可沒這麼幸運，常常會是改 A 改 B，而不只 A 和 B 沒做好，C, D, E…可能也跟著被改壞 😂…
先不拿 Side Project 的 production Code 動手，先以明顯有優化空間的程式碼（說白了就是寫得有點醜），作為範例來練習重構 💪。如此一來，可以比較清楚重構的概念與做法，才不會進去程式碼搞得一團糟。
範例是這樣的，以今天的記帳 App 來說，使用者在使用後覺得不錯，提了一個新的需求：想要計算「飲食」這個帳目的總金額，如下的使用者故事所述：
作為一名使用者，
我希望可以知道記帳部分，花了多少錢在吃的上面，
以便統計飲食種類的「消費總金額」
看起來蠻簡單的，剛好最近都在用 AI 幫忙產 Code ，手感有點生疏，剛好趁機試試看手寫一下。轉成程式碼，很直覺的就是從 History 列表，只拿其中種類「飲食」的數量。那我也「沒有多想」，就從用個 for loop 搭配 if 做判斷。
function calculateFoodCatogoryCount(historyList: HistoryItem[]) {
let foodCategoryTotalAmount = 0;
for (const item of historyList) {
if (item.category === "飲食") {
foodCategoryTotalAmount += item.amount;
}
}
return foodCategoryTotalAmount;
}
接下來寫測試驗證對不對，總不能說寫個 console.log 看輸出的值對不對而已，還是要寫個測試才安心。至於測試部分就直接請 AI 生成，AI 很主動地生成了好幾個測試項目，稍微檢查了一下，確實如我們預期的那樣去做驗證。
馬上跑測試來看看，全都通過了，很好！
describe('calculateFoodCategoryCount', () => {
it('should return 0 when the history list is empty', () => {
const historyList: HistoryItem[] = [];
const result = calculateFoodCatogoryCount(historyList);
expect(result).toBe(0);
});
it('should correctly calculate the amount for food category', () => {
const historyList: HistoryItem[] = [
{ amount: 100, category: '飲食', id: 1 },
{ amount: 50, category: '娛樂', id: 2 },
{ amount: 200, category: '飲食', id: 3 },
];
const result = calculateFoodCatogoryCount(historyList);
expect(result).toBe(300);
});
//...
})
事到如今，有觀眾可能已經「手癢」，覺得上面的主程式有點「醜」，既然已經有測試保護，那是不是該重構一下了？
但先別急，目前程式碼的複雜度看起來沒什麼大問題，就一個 for 和 if 而已，形塑重構作法的「力量」還不夠大，等下一個需求進來時，就會更清楚了。
接著呢，使用者因為最近這個月的「娛樂」開銷比較多，手遊課金課得多了一點，老婆叫他克制一點娛樂花費，但他也懶得算到底花了多少錢。不知道花多少錢，那也無從省起，不明白怎麼克制才好。
於是乎，使用者又提了另一個需求：「我希望可以計算娛樂種類的金額總數。」想到剛剛的這個 function 就有計算「飲食」種類的總開銷，那我直接「複製貼上」，多一個「娛樂」的種類不就了得？
因為多了一個種類的計算總額，輸出也就需要調整一下。
function calculateFoodCatogoryCount(historyList: HistoryItem[]) {
let foodCategoryTotalAmount = 0;
let entertainmentCategoryTotalAmount = 0;
for (const item of historyList) {
if (item.category === "飲食") {
foodCategoryTotalAmount += item.amount;
}
if (item.category === "娛樂") {
entertainmentCategoryTotalAmount += item.amount;
}
}
return { foodCategoryTotalAmount, entertainmentCategoryTotalAmount };
}
測試當然也跟著調整一下，改為驗證兩個值
describe('calculateFoodAndEntertainmentCatogoryCount', () => {
it('should return 0 when the history list is empty', () => {
const historyList: HistoryItem[] = [];
const result = calculateFoodAndEntertainmentCatogoryCount(historyList);
expect(result).toEqual({ foodCategoryTotalAmount: 0, entertainmentCategoryTotalAmount: 0 });
});
it('should correctly calculate the amount for food and entertainment category', () => {
const historyList: HistoryItem[] = [
{ amount: 100, category: '飲食', id: 1 },
{ amount: 50, category: '娛樂', id: 2 },
{ amount: 200, category: '飲食', id: 3 },
{ amount: 150, category: '娛樂', id: 4 },
];
const result = calculateFoodAndEntertainmentCatogoryCount(historyList);
expect(result).toEqual({ foodCategoryTotalAmount: 300, entertainmentCategoryTotalAmount: 200 });
});
//...
雖然功能沒問題，測試也 OK，但這樣一來一往，好像就有點「感覺」到說，如果下次我再多一個種類，啊…不就都要跟著動其他程式碼 😲 測試也要改…，這樣豈不是會花轟！
想了一想，這個寫法確實不是個好作法 🤔 但一時想不到有什麼問題，這時候重構的 Force 就出現了，我現在可以知道，照這方法繼續下去，程式碼會變得很醜陋，且變得很難修改調整！
首先分析程式碼，看一下程式碼可以優化的部分，大概有以下幾項：
剩下還有一些東西可以調整，但暫時想不太到，於是我先給了以上分析，請 AI 幫忙重構看看，提供了如下的指令
試著用 gpt-4o-mini 和 claude-3.5-sonnet 生成看看，聽聽看不同模型的意見：
gpt-4o-mini 的回答
你提到的重構點非常好！除了你提到的兩個方面，還有以下幾個可以考慮的重構建議：
1. 抽象化類別：可以將類別作為參數傳遞，這樣可以避免硬編碼類別字串，增加函數的靈活性。
2. 使用映射：可以使用一個物件來映射類別到其總和，這樣可以更容易地擴展和維護。
3. 分離計算邏輯：可以將計算邏輯提取到一個單獨的函數中，這樣可以提高可讀性和可測試性。
以下是重構後的代碼示例：
// components/AccountingAppForDemo/utils/calculateHistoryCount.ts
export function calculateCategoryCount(historyList: HistoryItem[], categories: string[]) {
const categoryTotals: { [key: string]: number } = {};
for (const category of categories) {
categoryTotals[category] = 0; // 初始化每個類別的總和
}
for (const item of historyList) {
if (categoryTotals.hasOwnProperty(item.category)) {
categoryTotals[item.category] += item.amount;
}
}
return categoryTotals;
}
// 使用示例
export function calculateFoodAndEntertainmentCatogoryCount(historyList: HistoryItem[]) {
return calculateCategoryCount(historyList, ["飲食", "娛樂"]);
}
claude-3.5-sonnet 的回答
您提出了很好的觀察。讓我們來討論一下這個函數的重構方案，並提供一些額外的建議：
1. 單一職責原則（SRP）：您說得對，這個函數確實違反了單一職責原則。
2. 硬編碼的類別：同樣正確，直接在函數中硬編碼類別名稱不是一個好做法。
3. 重複邏輯：calculateFoodCatogoryCount 和 calculateFoodAndEntertainmentCatogoryCount 中有重複的邏輯。
4. 命名問題：函數名稱中的 "Catogory" 拼寫錯誤，應該是 "Category"。
5. 類型安全：可以使用 TypeScript 的類型系統來增強代碼的安全性。
6. 靈活性：當前的實現不夠靈活，難以擴展到其他類別。
基於這些觀察，我們可以進行以下重構：
import { HistoryItem } from "../types";
type Category = "飲食" | "娛樂" | string;
export function calculateCategoryTotal(historyList: HistoryItem[], category: Category): number {
return historyList.reduce((total, item) =>
item.category === category ? total + item.amount : total, 0);
}
export function calculateMultipleCategoryTotals(historyList: HistoryItem[], categories: Category[]): Record<Category, number> {
return categories.reduce((result, category) => {
result[category] = calculateCategoryTotal(historyList, category);
return result;
}, {} as Record<Category, number>);
}
// 使用示例：
// const foodTotal = calculateCategoryTotal(historyList, "飲食");
// const totals = calculateMultipleCategoryTotals(historyList, ["飲食", "娛樂"]);
比較上面 2 種回答，很明顯的，claude-sonnet 給的回答，清晰且具體很多，還清楚地跟我說，’Catogory’ 拼錯了 😂
最後看了一下，覺得這個方案蠻好的，for loop 和其中的功能分開，相當的「單一職責」。
但我覺得還可以優化，認為「不必」特地把種類作為參數傳入，應該是用個 map 的資料結構，以種類作為 key ，將計算結果算出來即可。
再給了以下 follow up 指令進行後續重構：
以上述重構方向為基礎，我想要「一次算出所有種類」就好，2 個 function 都不必輸入 category 做為參數
claude 給了我這樣的結果：
import { HistoryItem } from "../types";
export function calculateAllCategoryTotals(historyList: HistoryItem[]): Record<string, number> {
return historyList.reduce((totals, item) => {
if (!totals[item.category]) {
totals[item.category] = 0;
}
totals[item.category] += item.amount;
return totals;
}, {} as Record<string, number>);
}
// 使用示例：
// const allCategoryTotals = calculateAllCategoryTotals(historyList);
// console.log(allCategoryTotals);
// 輸出可能如下：{ "飲食": 1000, "娛樂": 500, "交通": 300, ... }
看起來很棒，就直接 Apply 到程式碼，行數「驚人的」減少很多！
最後用「行內指令」，做點小調整，讓程式碼變得更簡潔，意圖性更強烈。
最後的最後，測試連跑都不用跑，用膝蓋想也知道會錯，因為整個主程式已經被大改了 😂
首先是先改 import 的函式，改成「只引入」一個主程式即可
import { calculateAllCategoryTotals } from "./calculateHistoryCount";
接著往下看，可以發現到我們原本是寫兩種測試群組，飲食和娛樂這兩個類型是分開測試，但現在不必這樣做了。因應主程式的調整，計算不同種類的總金額，已經變成「同一個功能，不同場景」，同樣都是計算「個別種類的金額加總」，只是分別是「飲食」和「娛樂」的場景。
甚至更進一步的，我覺得「計算不同種類」根本是同一件事，也沒必要分不同的種類。為何可以這樣說？
你就想嘛，如果每一個種類都要分別寫個測試項目，那每多一個種類，都要都一項，那其實不太合理。因為「種類」本身不是邏輯的一部分，我只需要確保，可以依照種類去加總即可，我並不關心種類到底是什麼。是只有 1 種，還是有 87 種，程式邏輯根本不關心，它的「職責」就是做好分種類加總而已。
稍微整理了一下想法，最後精簡而來的指令為：
幫我依照 @calculateHistoryCount.ts 去撰寫測試
- history list 為空
- history list 裡面的 category 有 8 種，共 20 筆資料，可以成功計算其加總
- 當我要取得 calculateAllCategoryTotals 計算結果中，沒有出現的種類時，會取得 undefined
為我產生了如下的程式碼，AI 相當很擅於「生成幻想」，這種準備假資料的工作，真的很厲害。
describe('calculateAllCategoryTotals', () => {
it('should return an empty object when the history list is empty', () => {
const historyList: HistoryItem[] = [];
const result = calculateAllCategoryTotals(historyList);
expect(result).toEqual({});
});
it('should correctly calculate totals for 8 categories with 20 items', () => {
const historyList: HistoryItem[] = [
{ amount: 100, category: '飲食', id: 1 },
{ amount: 200, category: '娛樂', id: 2 },
{ amount: 150, category: '交通', id: 3 },
{ amount: 300, category: '購物', id: 4 },
{ amount: 50, category: '醫療', id: 5 },
{ amount: 80, category: '教育', id: 6 },
{ amount: 120, category: '住宿', id: 7 },
{ amount: 90, category: '其他', id: 8 },
{ amount: 110, category: '飲食', id: 9 },
{ amount: 180, category: '娛樂', id: 10 },
{ amount: 130, category: '交通', id: 11 },
{ amount: 250, category: '購物', id: 12 },
{ amount: 70, category: '醫療', id: 13 },
{ amount: 100, category: '教育', id: 14 },
{ amount: 140, category: '住宿', id: 15 },
{ amount: 60, category: '其他', id: 16 },
{ amount: 95, category: '飲食', id: 17 },
{ amount: 175, category: '娛樂', id: 18 },
{ amount: 85, category: '交通', id: 19 },
{ amount: 215, category: '購物', id: 20 },
];
const result = calculateAllCategoryTotals(historyList);
expect(result).toEqual({
'飲食': 305,
'娛樂': 555,
'交通': 365,
'購物': 765,
'醫療': 120,
'教育': 180,
'住宿': 260,
'其他': 150
});
});
it('should return undefined for non-existent categories', () => {
const historyList: HistoryItem[] = [
{ amount: 100, category: '飲食', id: 1 },
{ amount: 200, category: '娛樂', id: 2 },
];
const result = calculateAllCategoryTotals(historyList);
expect(result['飲食']).toBe(100);
expect(result['娛樂']).toBe(200);
expect(result['不存在的類別']).toBeUndefined();
});
});
最後其實還可以做點小調整，像是 result['不存在的類別'] 這部分，還可以處理一下，其實應該是 return 0 比較合理。不過這就留給讀者自己發揮囉。
*備註：還有一個方法，就是將 historyList 轉為「物件」的形式，以 OOP 的角度去重構，以下提供其中一種寫法，供大家參考 :)
class HistoryListVo {
historyList: HistoryItem[];
constructor(historyList: HistoryItem[]) {
this.historyList = historyList;
}
getAllCategoryTotals(): Record<string, number> {
return this.historyList.reduce((totals, item) => {
totals[item.category] = (totals[item.category] || 0) + item.amount;
return totals;
}, {} as Record<string, number>);
}
getCategoryTotalAmount(category: string): number {
const allCategoryTotals = this.getAllCategoryTotals();
return allCategoryTotals[category] || 0;
}
}
先在「單純」的情境試試看重構，熟悉與練習之後，再到正式的專案做實際應用，如此一來比較不會慌，也比較不會被真實情境的複雜度給「嚇到」了。
撇除「幻想問題」的缺點，AI 在開發程式的各方面幫助確實都相當亮眼。
特別是目前最先進的商用 LLM 模型，Claude-3.5 在程式撰寫上表現優異，除了能提供建議的寫法，還能解釋為何應該該這樣做，無疑是現代學習程式的一大利器呀！
iThome鐵人賽
看影片追技術