Sys.StringBuilder里的Bug
2007-02-13 02:35 by 老赵, 6488 visits他们肯定没有做好Code Review。
Sys.StringBuilder类源码
话不多说，首先我把Sys.StringBuilder类的源代码完整的贴出，然后根据代码来寻找问题：
Sys.StringBuilder = function Sys$StringBuilder(initialText) { /// var e = Function._validateParams(arguments, [ {name: "initialText", mayBeNull: true, optional: true} ]); if (e) throw e; this._parts =
(typeof(initialText) !== 'undefined' && initialText !== null && initialText !== '') ? [initialText.toString()] : []; this._value = {}; this._len = 0; } function Sys$StringBuilder$append(text) { /// var e = Function._validateParams(arguments, [ {name: "text", mayBeNull: true} ]); if (e) throw e; this._parts[this._parts.length] = text; } function Sys$StringBuilder$appendLine(text) { /// var e = Function._validateParams(arguments, [ {name: "text", mayBeNull: true, optional: true} ]); if (e) throw e; this._parts[this._parts.length] = ((typeof(text) === 'undefined') || (text === null) || (text === '')) ? '\r\n' : text + '\r\n'; } function Sys$StringBuilder$clear() { if (arguments.length !== 0) throw Error.parameterCount(); this._parts = []; this._value = {}; this._len = 0; } function Sys$StringBuilder$isEmpty() { ///if (arguments.length !== 0) throw Error.parameterCount(); if (this._parts.length === 0) return true; return this.toString() === ''; } function Sys$StringBuilder$toString(separator) { /// /// var e = Function._validateParams(arguments, [ {name: "separator", type: String, mayBeNull: true, optional: true} ]); if (e) throw e; separator = separator || '';
var parts = this._parts; if (this._len !== parts.length) { this._value = {}; this._len = parts.length; }
var val = this._value; if (typeof(val[separator]) === 'undefined') { // Need to remove empty elements before joining in the separator case if (separator !== '') { for (var i = 0; i < parts.length;) { if ((typeof(parts[i]) === 'undefined') ||
(parts[i] === '') || (parts[i] === null)) { parts.splice(i, 1); } else { i++; } } } val[separator] = this._parts.join(separator); } return val[separator]; } Sys.StringBuilder.prototype = { append: Sys$StringBuilder$append, appendLine: Sys$StringBuilder$appendLine, clear: Sys$StringBuilder$clear, isEmpty: Sys$StringBuilder$isEmpty, // separator may be null, to match behavior of ECMA Array.join(separator) and // .NET String.Join(separator, value) toString: Sys$StringBuilder$toString } Sys.StringBuilder.registerClass('Sys.StringBuilder');
Sys.StringBuilder类的Bug分析与重现
Sys.StringBuilder方法的实现很简单，只是把每一次append的内容添加到_parts数组里，在toString时调用数组的join方法来生成一个新的字符串，以次避免过多字符串拼接造成的性能损失。其中Bug的关键就在于它的toString方法，我们自己来分析一下它的实现。
toString方法的作用是把separater作为分隔符，连接StringBuilder内已经添加的部分。它的作用和.NET Framework里String.Join(string separator, string[] value)的功能非常相似。首先，如果用户没有提供separater，则默认separator为空字符串。然后判断_len与_parts数组的长度是否相同，如果不同，则表示_parts被修改过了，于是将作为缓存的_value字典清空并保留新的_len值。如果separator不为空字符串，则遍历_parts数组，清除其中所有为空的元素。这个操作不会影响结果，但是能够提高join方法的性能。最后将调用_parts数组的join方法生成一个新的字符串，并以separator作为key保存在_value数组中。这样，如果Sys.StringBuilder没有改变的话，使用相同的separator调用toString方法则会直接从_value字典的缓存中获取，而避免了再次调用join方法造成的性能损失。
这只是我对于这段实现的“意图”的一个理解，因为这段实现有着非常严重的Bug。大家有没有觉得这段代码有些奇怪？如果_len与_parts数组的长度不同时，则会认为_parts数组被改变了（这“似乎”没错）。但是在重新设定了_len的数值之后，接下来的代码又可能修改了_parts数组（用于清空其中的空元素），这样即使Sys.StringBuilder的内容不被外界改变时，_parts数组的长度也变化了，这造成了_len的数值与_parts.length不同，缓存失效，又必须调用join方法了——没错，甚至会重新遍历_parts数组。
下面的代码就直接说明了问题：
var sb = new Sys.StringBuilder(); for (var i = 0; i < 5; i++) { sb.append(i); sb.append(null); } alert(sb.toString('|')); alert(sb.toString('|'));
首先，我们向StringBuilder中添加部分空元素，然后连续调用两次toString方法，传入同样的separator。如果跟踪一下代码就会发现，第二次调用toString方法时并没有利用到缓存，而且重新遍历了_parts数组。虽然生成了相同的结果，但是与它设计的本意违背了，它的缓存“无缘无故”地失效了。
如果说，上面的问题只能说明这个Bug使Sys.StringBuilder没有符合Sys.StringBuilder的设计初衷，那么我下面构造的一个场景就表明这个Bug的严重性了，它能够使Sys.StringBuilder的使用得到错误的结果。如下：
var sb = new Sys.StringBuilder(); for (var i = 0; i < 5; i++) { sb.append(i); } sb.append(null); alert(sb.toString('|')); // "0|1|2|3|4" sb.append('abc'); alert(sb.toString('|')); // 依旧是"0|1|2|3|4"？
很明显，两次alert得到的结果都是"0|1|2|3|4"完全是错误的。我只是简单的利用了toString方法中判断_parts数组有没有被更新的错误方法——怎么可以使用_len记录_parts数组长度的同时又减少了_parts数组的元素呢？在上面的代码中，我向StringBuilder里添加了一个将会被去掉的null元素，然后在调用toString方法之后重新append一个新的元素以“补充”_parts数组。“愚蠢”的StringBuilder这下认为Sys.StringBuilder没有任何改变，从_values字典中读取了被缓存的结果，这自然是错误的。
RC版本中的StringBuilder居然是正确的
RC版本中的Sys.StringBuilder类的实现居然是正确的，这不是天方夜谭，我们来看一下StringBuilder在RC中的做法：
function Sys$StringBuilder$append(text) { /// var e = Function._validateParams(arguments, [ {name: "text", mayBeNull: true} ]); if (e) throw e; if (typeof(text) !== 'undefined' && text !== null && text !== '') { this._parts[this._parts.length] = text.toString(); this._value = {}; } } function Sys$StringBuilder$toString(separator) { /// ///var e = Function._validateParams(arguments, [ {name: "separator", type: String, mayBeNull: true, optional: true} ]); if (e) throw e; separator = separator || ''; if (typeof(this._value[separator]) === 'undefined') this._value[separator] = this._parts.join(separator); return this._value[separator]; }
多简单优雅的实现，每当调用append方法时，将会直接判断text的有效性（而不是在调用toString方法时去除无效元素），在确认有效之后则添加至_parts数组中，然后将_value字典，也就是把缓存清空。在调用toString方法时，则不需要去除parts数组中的无效元素，也可以非常合理的利用缓存。可以看出，在RC中，并没有_len的作用。
那么为什么在RTM时Sys.StringBuilder反而被修改了呢？在这里，我指的是为什么去“修改”，而不是为什么“改错了”。我认为，他们只是为了避免在调用append方法时将_value字典设为新的对象吧。按照RC中的实现，如果调用了1000次append方法，则会生成1000个多余的对象，这的确是种浪费。但是，他们只要这样改不就可以了吗？如下：
function Sys$StringBuilder$append(text) { if (typeof(text) !== 'undefined' && text !== null && text !== '') { this._parts[this._parts.length] = text.toString(); this._value = null; } } function Sys$StringBuilder$toString(separator) { separator = separator || ''; if (!this._value) this._value = {};
if (typeof(this._value[separator]) === 'undefined') this._value[separator] = this._parts.join(separator); return this._value[separator]; }
还好，现在的Sys.StringBuilder的Bug并不是那么容易遇到的，在整个Microsoft AJAX Library的代码中还没有遇到过提供separator的toString方法调用。不过这一点并不能掩盖这个错误，如果您需要使用Sys.StringBuilder的话，也请注意一下这个Bug。
总之，在Sys.StringBuilder问题上，他们一定没有做好Code Review。
这个Bug让人啼笑皆非啊。还好的确不是能够轻易遇到的。