海克力士是希臘神話裡出了名的大力士，有一次，天后赫拉不知道用了什麼伎倆讓海克力士陷入瘋狂，等海克力士清醒過來，才發現自己竟然親手殺死了孩子。

而為了贖罪，他按照神諭的指示，開始替歐律斯透斯完成一連串幾乎不可能的任務，也就是後來著名的「十二項試煉」。

交給他的試煉，幾乎全是普通人幾乎不可能辦到的事情...對付尼米亞獅、斬殺九頭蛇，聽起來應該都是很難纏的怪物吧？

但是其中有一項試煉，居然是讓他去清牛棚。

而且不是慢慢清，是要在一天之內，把整座牛棚全部清乾淨。

牛棚的主人是厄利斯國王奧革阿斯，他養了一大群牛，而這座巨大牛棚已經不知道多久沒有好好清理，裡面的糞便堆積到根本不是多找幾把鏟子就能解決的程度。

要在一天內清完？就算海克力士力氣再大，一鏟一鏟挖也不可能來得及。

但海克力士根本沒打算拿著鏟子慢慢清，而是觀察後，直接在牛棚的地基挖開入口與出口，再把附近的阿爾菲俄斯河與佩奈俄斯河改道，讓河水直接穿過整座牛棚，把糞便全部沖出去，結果真的在一天內清乾淨了。

大力出 Trouble

先別管海克力士力氣到底有多大，我們在故事中卻可以看到他靈機應變的能力，他先是看出一鏟一鏟根本來不及，乾脆改變河水流過牛棚的路徑，這告訴我們有時候只靠蠻力是不夠的，我們需要的是策略，休想一次產這麼多程式碼 XD

今天想介紹的內容也差不多，不是看到幾行可疑的程式碼就直接刪掉，而是得先弄清楚資料怎麼流，再找一個入口，確認這股水最後真的沖到了我們想清理的位置。

在前面的章節中，我們學到當客戶已經在發火、修正時間又非常有限時，我們可以利用 Sprout 和 Wrap 技術做處理，盡量避免直接碰觸難以現在就修改的舊邏輯。

但如果今天遇到的問題不是「少了一段功能」，而是舊程式裡存在一段明確錯誤、幾乎可以確定應該刪掉的邏輯呢？

看起來似乎很簡單：

找到錯誤的程式碼，刪掉，結案。

圖片擷取自網路

問題是，我們即使知道刪掉這段程式可以修正眼前的 Bug，也無法確定它是否會對這個 Method 的其他流程造成影響。

「那就補測試啊！」

麻煩的是，這個 Method 可能有上千行，重點是我們就沒有時間，我們不可能先替它補完所有特徵測試，再開始修 Bug，跟前面的理念一致，我們必須先想辦法先建立一個夠小、夠快，而且能保護這次修改的回饋機制。

Bug 藏在哪裡？

在今日範例中，ProcurementService.UpdateProcurement() 是一個有上千行的 Monster Method，負責處理採購單的各種異動。

其中有一段流程負責刪除標記要刪掉的採購項目。

前端會在 Detail 資料上標記狀態，讓後端知道哪些資料是新增、修改或刪除，而後端一開始會先把可能需要刪除的資料放進 deleteLineList，接著在處理新增與修改的過程中，逐步把已經處理完成的資料從清單中移除，最後留在清單裡的，才是真正需要刪除的資料。

除此之外，每一筆採購項目底下還有零件、備註、證書、加工規格等附屬資料，聽起來不算太複雜，但當所有流程全部塞在同一個 Method 時，實際 Debug 起來就沒有那麼輕鬆了。

這次的 Issue 內容是：

刪除採購項目後，主資料被刪除了，但附屬表的資料沒有一起刪除。

順著中斷點往下追程式碼，我們找到了這一段：

// 433 行
if (deleteLineList.Count > 0)
{
    foreach (var line in deleteLineList)
    {
        deleteLineNoteList =
            deleteLineNoteList.Where(
	            x => x.Line_sn != line.Line_sn).ToList();

        deleteLinePartList =
            deleteLinePartList.Where(
	            x => x.Line_sn != line.Line_sn).ToList();

        deleteLineCertList =
            deleteLineCertList.Where(
	            x => x.Line_sn != line.Line_sn).ToList();

        if (model.Procurement_type == ProcurementType.Processing)
        {
            deletePriceDetailList = deletePriceDetailList.Where(
	            x => x.Line_sn != line.Line_sn).ToList();

            deleteProcessSpecList = deleteProcessSpecList.Where(
	            x => x.Line_sn != line.Line_sn).ToList();

            deleteVendorSupplyList = deleteVendorSupplyList.Where(
	            x => x.Line_sn != line.Line_sn).ToList();
        }
    }

    foreach (var line in deleteLineList)
    {
	    isSuccess = dal.DeleteLine(line.Line_sn);
        if (!isSuccess)
        {
	        res.ErrMsg = "採購明細刪除失敗，資料庫操作異常。";
	        return res;
        }
    }
}

這六行 .Where(...) 看起來很可能是 Copy and Paste 造成的結果，deleteLineList 裡放的是即將被刪除的採購項目，但程式卻把這些項目所對應的附屬資料，從待刪除清單裡排除了。

例如，要刪除的採購項目 Line_sn 是 1，程式卻執行：

deleteLinePartList =
    deleteLinePartList.Where(x => x.Line_sn != 1).ToList();

結果就是所有屬於這筆採購項目的零件資料，都被移出 deleteLinePartList。

等到接下來真正執行刪除時，收到的已經是空清單，自然就什麼都不會刪除。

這裡要注意一件事，我們在這段程式碼中，看不到附屬資料最終跑去哪裡了...這裡只刪除主單資料，附屬表卻不在這裡一起被刪掉，要嘛有人做傻事，要嘛就是中間他又不知道做了什麼事情。

呼叫 dal 做刪除的地方和目前這段程式碼之間，中間夾著一大堆我們不知道在幹嘛的流程，而且我們根本還沒讀完這些程式碼，這些清單被 Filter 之後，到底有沒有被其他地方再次修改，從目前來說根本無從判斷。

但是順著中斷點確實看起來只要把這六行拿掉，讓附屬資料留在待刪清單裡，不就解決了嗎？

當然可以，但是！我們也已經知道在開始前要先做特徵測試了，那麼：

測試要怎麼補？

開門

經過我們一步步查驗後，看到問題似乎是出在這六段篩選邏輯，但它離實際操作 dal 的距離很遠，根本沒有辦法直接做驗證。

因此，第一步我們並不打算改變它的行為，仍然要用 IDE 的 Extract Method 將這段邏輯給抽出來：

protected List<LineNote> FilterDeleteLineNotes(
	List<LineNote> deleteLineNoteList,
	int lineLineSn)
{
    return deleteLineNoteList.Where(
	    x => x.Line_sn != lineLineSn).ToList();
}

// ...其餘也採用相同方式處理。

原本迴圈中的程式碼則改成：

foreach (var line in deleteLineList)
{
    deleteLineNoteList =
        FilterDeleteLineNotes(deleteLineNoteList, line.Line_sn);

    deleteLinePartList =
        FilterDeleteLineParts(deleteLinePartList, line.Line_sn);

    deleteLineCertList =
        FilterDeleteLineCerts(deleteLineCertList, line.Line_sn);

    if (model.Procurement_type == ProcurementType.Processing)
    {
        deletePriceDetailList =
            FilterDeletePriceDetails(deletePriceDetailList, line.Line_sn);

        deleteProcessSpecList =
            FilterDeleteProcessSpecs(deleteProcessSpecList, line.Line_sn);

        deleteVendorSupplyList =
            FilterDeleteVendorSupplies(deleteVendorSupplyList, line.Line_sn);
    }
}

這一步只是 Extract Method，程式行為還沒有改變，不會花上我們太多的時間，當然，將原本的程式碼抽出來之前，還是得先確認抽取的範圍是否合理。

如果只是看到一段可疑程式碼，就直接認定它一定是 Bug，也不見得可靠而且更花時間，最好善用中斷點再順著資料流多看幾層，確認這些清單在前後流程中的角色，如果真的完全看不懂，也可以找熟悉這段程式碼的團隊成員一起確認，啊妥善運用 AI 也是個很好的選項，我認為處理 Legacy Code 確實很吃經驗。

和它相處久了之後，我們會慢慢培養出一種感知能力，哪些程式碼只是看起來奇怪，哪些程式碼則真的散發出「這裡有問題」的氣息，雖然這個能力很難量化就是了 XD

改道

接著，我們建立一個測試用子類別，將 protected 轉接成 public，讓測試可以直接呼叫。

public class TestableProcurementService()
    : ProcurementService(new Mock<IServiceProvider>().Object)
{
    public List<LineNote> CallFilterDeleteLineNotes(
        List<LineNote> notes,
        int lineSn)
        => base.FilterDeleteLineNotes(notes, lineSn);

    // ... 其他 5 個 Method 做法一致
}

接著寫 Characterization Test，記錄目前程式的實際行為：

public class UpdateProcurementTests
{
    private readonly TestableProcurementService _sut = new();
    
    [Fact]
    public void 應回傳處理後的LineNote清單()
        => AssertFiltersLineSn(
            [new LineNote { Line_sn = 1 }, new LineNote { Line_sn = 2 }],
            input => _sut.CallFilterDeleteLineNotes(input, lineSn: 1));

    // ... 其餘省略
	
    private void AssertFiltersLineSn<T>(
        List<T> input,
        Func<List<T>, List<T>> filterAction) where T : class
    {
        var expected = input[1];
        var result = filterAction(input);
        Assert.Single(result);
        Assert.Contains(expected, result);
    }
}

這些測試跑起來應該會是綠燈，它們證明了目前的 Filter 會排除 Line_sn 相同的附屬記錄，同時保留其他 Line_sn 的資料。

但要注意，這還不能完全證明整個 Bug 的因果關係。

它只能確認：

我們對這一小段程式碼的理解沒有偏差。

至於資料是否真的因此沒有傳進 dal，仍然需要範圍更大的測試來確認，而這件事情我們會在後續來做。

沖水

確認完既有行為後，我們不再需要保留這個舊行為，而是將這個測試轉成描述 Hotfix 後應有的行為：

private void AssertFiltersLineSn<T>(
    List<T> input,
    Func<List<T>, List<T>> filterAction) where T : class
{
    var result = filterAction(input);
    Assert.Equal(input, result);
}

現在執行測試，理所當然會得到紅燈。

這就是為什麼我們不直接刪掉 foreach，因為現在我們手上的資訊還不完整，還在探勘。

於是接下來直接 no-op 取得綠燈，把「Filter 行為已確認改正」這件事固定下來，才能站穩腳步繼續往外擴大測試範圍，等到下一層測試也到位了，才是有足夠信心刪除整段程式碼的時候。

接著把 Method 改成 no-op，直接回傳原本的清單：

protected List<LineNote> FilterDeleteLineNotes(
	List<LineNote> deleteLineNoteList,
	int lineLineSn)
        => deleteLineNoteList;

// ... 其他 5 個同理

再次執行測試，綠燈。

到這裡，我們完成了一個很小的紅綠循環：

1. 用 Characterization Test 確認現況。
2. 將它轉成描述 Hotfix 後應有的行為。
3. 修改程式碼讓測試通過。

目前我們只是把候選修正的局部行為固定下來，還不能說整個 Bug 已經被保護，這時候可能會有同事看著這些 Method 問：

「你是在搞笑嗎？這些 Method 裡什麼都沒做，為什麼不直接刪掉？」

「嗯？」，這個問題完全合理。

幹嘛不直接把整個 foreach 刪掉？

實際上，這段 foreach 最終很可能就是應該被刪除。

Legacy Code 最令人害怕的地方，往往不是修改本身，而是我們不知道這段程式還偷偷影響了哪些地方，但是把六個 Filter Method 改成 no-op，確實不是漂亮的最終設計。

下一位維護者很可能會想：

這個 Method 根本什麼都沒做，刪掉算了。

如果這段 foreach 裡真的只有這六個 Filter，那麼從 production 行為來看，「六個 Filter 全部改成直接回傳原清單」和「刪掉整個 foreach」基本上是等價的，no-op 本身並沒有神奇地比較安全。

真正增加安全性的，是前面建立的回饋，目前能快速寫下較窄的測試，直接保護住的就是這些剛抽出來的 Filter，而修改測試變紅燈，再把它們改成 no-op，我們可以確認 Hotfix 確實讓附屬資料都乖乖地留在待刪清單裡，而不是憑感覺移除可疑程式碼，保留 no-op 只是 緊急部署 的中繼站，讓這組測試暫時還有明確的檢查點，並不是在說 no-op 比刪除 foreach 更可靠。

所以我們確實會為了取得快回饋而暫時留下的一道傷疤，不過那也沒關係，我們先透過它解決 Hotfix，等我們調查的越來越清楚，那麼再逐漸把測試範圍往外擴大就好了。

等到我們最後留下來的，不管是用什麼方式，只要是能描述真正的意圖：

當採購項目被刪除時，它的附屬資料也必須進入刪除流程。

那我們就把整段無效的 foreach 刪掉，接著再把前面為了 Hotfix 暫時建立的 no-op，以及只關注這些 Method 的測試一起清理掉了。

不是不到，是時候未到。

測試也是團隊的記憶

人腦其實非常不可靠，處理完這個 Hotfix 後，我們通常立刻就得去處理下一個問題，至於什麼時候有空回來補齊測試，沒有人知道。

等到幾個星期或幾個月後再次打開它，我們很可能已經忘記：

• 當初為什麼懷疑這段程式？
• 為什麼這個 Method 什麼都沒做？
• 為什麼不能把這段邏輯加回來？
• 當時修正的是哪一個實際問題？

如果程式碼和測試都沒有留下線索，我們十之八九得重新閱讀一次整個流程，測試不只驗證程式是否正確，在 Legacy Code 裡，也可以保存團隊對系統的理解，避免這份理解隨著時間而扭曲或消失。

比起註解寫著：

// 這裡不能過濾，否則附屬資料不會被刪除。

一個接受過考驗的測試，我是覺得比較有說服力。

結論

這次的做法可以整理成幾個階段：

1. 找出疑似造成問題的最小程式區塊。
2. 透過 Extract Method 建立可測試性。
3. 用 Characterization Test 記錄目前行為。
4. 將測試轉成描述 Hotfix 後應有的行為，取得紅燈。
5. 修改讓測試綠燈。
6. 繼續把測試範圍往外擴大。
7. 有了更可靠的敘述後，刪除不再需要的部分。

所以，在緊急情境下，面對一段幾乎確定應該被刪除的程式碼，我們不必只能在以下兩個選項之間二選一：

• 花很多時間做單元或整合測試。
• 直接刪掉程式，部署祈禱。

我們可以先用極小而保守的修改建立測試接縫，快速取得回饋，等眼前的火滅掉後，再逐步擴大測試範圍，最後把暫時留下的傷疤清理乾淨，就像奧革阿斯的牛棚，重點不是拿到一把更大的鏟子，而是先觀察要怎麼讓水流進來，否則改了半天，只是又累又髒而已。

明天我們繼續看：看似測不了的東西，我們怎麼辦？

Reference

• The key points of Working Effectively with Legacy Code，understandlegacycode.com
• Testing Protected Methods in C#，Damir's Corner
• Extract Method refactoring，Zaengle
• Legacy Seam，Martin Fowler