字里行间有学问——从一个方法的吐槽开始，说说几个编码实践

这两天小小的维护了下一个历史遗留的基于.net 1.1开发的后台程序，加几行代码上去，这场景需要将一个ArrayList类型的返回值转换为string[]传给另一个方法，听“老人”说，已经写了个方法来干这事，OK，那就用之：

ArrayList list = GetList(); // 返回ArrayList的方法，原名不是这个
int count = list.Count; // 下文还做它用，用变量存储

// ** 就加了这么两行代码 **
string[] array = StringHelper.List2Array(list);
ProcessArray(array); // 接收string[]的方法，原名不是这个

// ... 中间省略

for (int i = 0; i < count; i++) {
     // do somthing with list[i]
}

可一运行，就“不对劲”了，为何ArrayList里5个字符串到ProcessArray接收的数组里剩4个了？怎么后面的for循环还能抛出个ArgumentOutOfRangeException来？
实在纳闷，于是几经周折，找来了List2Array方法的源码：

/// <summary>
/// 列表转为字符串数组
/// </summary>
public static string[] List2Array(ArrayList list)
{
    if (list == null)
        return null;
    list.TrimToSize();
    for (int i = 0; i < list.Count; )
    {
        string str = list[i] as string;
        if (str == null || str.Length == 0)
        {
            list.RemoveAt(i);
               list.TrimToSize();
        }
        else
        {
            i++;
        }
    }
    string[] strArray = null;
    if (list.Count > 0)
    {
        strArray = new string[list.Count];
        for (int i = 0; i < list.Count; i++)
        {
            strArray[i] = list[i].ToString().Trim();
        }
    }
    return strArray;
}

看了代码，不到30行，却有一种五味陈杂的感觉。安抚了心中奔腾的马儿们，冷静下来，用文字总结了一下。

方法中在for循环前和循环中对于ArrayList的TrimToSize操作造成了额外的性能开销，而最终效果等同于在for循环后做一次TrimToSize。
奇怪的是，这不是我第一次看到这样用TrimToSize了，而且还是在不同的公司里，是否意味着TrimToSize方法在命名或文档上具有误导性，可能让开发者以为不断做此操作能够获得提升性能？
程序跑在一个内存充裕的环境中，而运行时传入的ArrayList大小通常不超1000，并且都是用过即丢，我并没有找到一个需要TrimToSize的理由，可以认为，这里的TrimToSize是多余的。

副作用

该方法修改了输入值，将输入的ArrayList实例中不是字符串或是空字符串的元素移除了，从而产生了副作用，导致了调用代码下文发生ArgumentOutOfRangeException，因为ArrayList的长度发生了变化。
副作用来自于对输入值的修改，这立刻让我想到了函数式编程的概念。对比此时的场景需求，我需要的正是个函数——输入值ArrayList，输出string[]——它并不依赖输入输出值外的其他状态，完全可以是个纯函数。

职能不单一，它做了太多的事情

要把这个方法方法改成纯函数，就不能修改输入值，这就纠结了，它在修改输入值上做了N件事。
要怎么处理，来逐一考虑：

将非字符串元素从ArrayList里移除——怎么处理这个问题，放在下文说明。

将空字符串从ArrayList里移除——此操作并不影响ToArray的实现，一码事是一码事，这个操作应该在别的地方做：可以是调用者所在的位置由方法调用者自行处理；若真的提供一个独立的方法（也应该是函数）来实现，它的签名可以是：

ArrayList NonEmptyElements(string[] array);

或者，应用上Linq的思想，也可以是：

IEnumerable NonEmptyElements(string[] array);

对ArrayList做TrimToSize——上文已经提到，TrimToSize就不用了，这个操作直接删掉罢。

在将复制进数组的字符串的前后空白字符移除——和前面的“将空字符串从ArrayList里移除”操作一样，应该被分离出去。

“垃圾进，垃圾出”

从代码可以看出该方法使用的“垃圾进，垃圾出”（GIGO）的做法：若list中的元素数量为0，或者仅包含null、非字符串、空字符串，则返回null。
在我们的习惯中，元素数量为0的ArrayList转为数组，也应该是一个长度为0的数组，而方法返回null，并不符合习惯，可能导致意外的发生，比如导致之后的代码访问类成员（比如Length属性）时发生空指针异常（NullReferenceException）。而对于仅包含null、非字符串、空字符串的集合返回null，则可以认为是一种不正确的“垃圾”判定，很有种越俎代庖的意味，它想当然得认为这些元素对于调用者是没有用的，也仍然犯了职能不单一的问题。

如果产生了“垃圾”，最终总得有个地方来接收和处理它们，为何不就近处理，让“垃圾”进得来、出不去，这就不得不提防御式编程了。《代码大全》中的一个章节便详细介绍了防御式编程。
对应上面的场景，我们需要“防住”什么——非字符串元素。非字符串元素如何转为字符串呢，ToString？若如此，就又越俎代庖了——替调用者决定了怎么转换。那既然这个方法里不能决定如何转换非字符串，那就只能“防住”它了，抛出个异常吧。

命名

应该给方法一个意义明确的能够说明其作用的名字，这不仅仅帮助后来的使用者更好的理解方法的作用，也让编码者获得了一个反思的机会，因为命名需要程序员花费不少脑力去思考，要不怎么说命名是程序员最头疼的事情呢。
List2Array并没有说明这个方法到底做了什么，如果确实需要这个方法，对应其功能，它的名称或许应该是：

TrimListAndRemoveNullOrEmptyStringsFromListThenTrimToArray

如此命名后，我相信方法的编写者自己就感觉到了代码异味，就可能回头调整重构，从而避免今天的这个小“悲剧”的发生。反之，使用的名称越模棱两可——比如ProcessData这种万精油——那么方法就越有可能是个大杂烩。

修改后的结果

综合上面的内容：

纯函数；
职能单一；
防御式编程；

我们得到修改后的方法：

public static string[] ToArray(ArrayList list)
{
    if (list == null)
        return null;

    string[] array = new string[list.Count];

    for (int i = 0; i < list.Count; i++)
    {
        // 防住非字符串元素，使用强制转换，对非字符串元素抛出InvalidCastException
        array[i] = (string)list[i];
    }

    return array;
}

写完后，可以发现它和ArrayList的内置ToArray方法功能是一致的，我们需要的其实只是这么一行代码：

string[] array = list.ToArray(typeof(string));

而我们发现上面自己写的ToArray方法从一开始就用不着写它。

另外，从.net2.0开始，泛型集合的引入解决了集合元素类型不明确的问题。再之后的.net3.5引入Linq后，要获得原方法的返回值就更简单了（不过没有修改原list的功能）:

string[] array = list.OfType<string>()
    .Where(x => !string.IsNullOrWhiteSpace(x))
    .Select(x => x.Trim())
    .ToArray();

总结

分析List2Array方法被编写出来的原因，可能是开发者对于.net标准库还不够熟悉，也可能是开发者希望一个调一个方法就能搞定一堆事情。

标准库的方法多是经过精心设计的，已经提供了诸多基础操作，了解并灵活使用它们，避免重新造轮子。在添加一个公共方法时，问自己：我真的需要写它吗？在调用一个方法时，问自己：我真的需要用到它吗？尤其工具库的方法，被不同开发者复用的概率最大，被用得越多，就越需要在意方法是不是简洁高效，有没有什么奇怪的行为、要求使用者遵守某些奇怪的约定，需要注意的细节过多时，可能带来的就不是效率的提升而是各种诡异的bug了。

写这一个方法事小，可一行行代码，都包含了许多编程思想与最佳实践，需要字斟句酌，慎之又慎。

最后，2014年即将到来。Happy New Year!

posted on 2013-12-31 22:59 codeyeast 阅读(237) 评论(0) 收藏举报

刷新页面返回顶部

codeyeast