分享我前幾天犯下的 LINQ 低級錯誤。

程式需求是從外界拿到一個型別為 Dictionary<string, string> 的 IP 對應表,我想跑迴圈檢查其中 IP 地址字串是否都合法,而專案裡剛好有個 ParseIp() 函式是用 IPAddress.TryParse() 解析傳回 IPAddress。因此,只要對 Dictionary 跑迴圈逐一用 ParseIp() 檢查抓出無效就成了,我慣用的寫法有兩種:

  1. dict.Values.ToList().ForEach(o => { ...檢查... })
  2. foreach (var ip in dict.Values) { ...檢查... }

突發奇想,不如用 Select() 寫成 dict.Values.Select(o => ParseIp(o)) 一樣會跑迴圈轉換每個字串,程式碼又比上面再簡潔幾個字元。(在 jQuery 對集合跑迴圈,我也愛用 $.map() 代替 $.each(),理由是 $.map(array, function(element) ) 處理函式宣告一個參數接入元素就好,而 $.each(array, function(index, element) ) 必須有 index, element 兩個元素,用 $.map() 寫少打幾個字,反正在 fucntion(element) 不 return 結果,只執行迴圈邏輯,純粹當成 $.each() 用。

[2020-05-10 補充] 以上提到用 Select() 取代 ToList().ForEach()/foreach() 及 用 $.map() 取代 $.each() 的技巧,好寫好用也可正確執行,但對於程式的可讀性是有害的。關於可讀性的重要性,在 閒聊 - 「好程式」跟你想的不一樣! 初讀「重構」有感,當今軟體開發的主流思維裡,程式可讀性甚至應該要凌駕於簡潔、效率之上,理由是低可讀性程式的維護成本常遠超過想像。有些取巧寫法固然能少打幾個字,但可能造成程式碼意圖混淆,導致後人誤解改錯,並不值得鼓勵。換言之,本文範例提到用 Select() 檢查,原理正確,執行結果無誤,但嚴格來說並不能稱為良好的寫法,建議仍應回歸 ForEach/foreach,或者改用 All() 或 Any()。關於這部分,我有另外寫了一篇說明

範例程式如下,我故意在 Dictionary 中摻入中文字串測試會不會被抓出來:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;

namespace Pitfalls
{
    class Program
    {
        static IPAddress ParseIp(string ip)
        {
            Console.WriteLine($"[DEBUG] Check {ip}");
            IPAddress r;
            if (IPAddress.TryParse(ip, out r)) return r;
            throw new ApplicationException(
                $"{ip} 不是有效 IP 地址");
        }
        static void Main(string[] args)
        {
            var rawData = new Dictionary<string, string>()
            {
                ["IP1"] = "127.0.0.1",
                ["IP2"] = "::1",
                ["BAD"] = "喂! 我不是 IP 啦"
            };
            try
            {
                rawData.Values.Select(o => ParseIp(o));
                Console.WriteLine("資料有效");
            }
            catch (Exception ex)
            {
                Console.WriteLine($"資料無效 - {ex.Message}");
            }
            Console.ReadLine();
        }
    }
}

跑了程式,登楞! 居然沒抓出亂打的 IP 地址。刻意在 ParseIp() 埋了 Console.WriteLine() 偵錯也沒顯示,這代表 ParseIp() 完全沒被執行:

一度我還懷疑是因為我沒用參數接收 Select() 的結果,這段程式被 Compiler 誤判沒用整段省略,用 dnSpy 檢查證明 C# 編譯器並不會胡亂自做主張,是我多疑了。又查了一陣子我才驚覺 - 是自己犯傻了,竟忘了 LINQ 有延遲執行的特性,在 Select() 的當下程式還不會執行,要等 .Count()/.ToList()/.ToArray() 才會開始跑迴圈。由於我不在乎資料,.Count() 比較省資源,加上後程式就正常了。

另外,LINQ 延遲執行特性也可以反過來應用,若沒必要跑完整個迴圈,可以優先考慮用 .FirstOrDefault()、.Any() 等方法,一旦符合條件就結束輪迴,不必傻傻跑完,會比先 ToList()/ToArray()/ForEach() 處理效率再好一些。

【延伸閱讀】

A real case of program bug caused by ignoring LINQ deferred execution feature.


Comments

# by ByTIM

題外補充: 我記得TryParse可以內嵌宣告,如 IPAddress.TryParse(ip,out IPAddress r) 可以讓程式碼更簡潔,但不保證可讀性好不好!

# by Min

大大,這邊少一個括弧喔!! rawData.Values.Select(o => ParseIp(o);

# by Jeffrey

to ByTIM,謝謝補充。out 內嵌變數宣告要 C# 7.0 才支援 ( https://docs.microsoft.com/zh-tw/dotnet/csharp/whats-new/csharp-7#out-variables ),我工作上有時要避免,目前還沒養成習慣,呵。

# by 卡比

比較似 IEnumerable 嘅特性,最近中過一次招

Post a comment