Aggregate Bug
gerneio opened this issue · 4 comments
Comparing the two sets of code (C# vs JS), the Aggregate functions aren't evaluating in the same way:
C# (https://dotnetfiddle.net/57YHxA)
var res1 = new object[] { 1, "2", "3" }.Aggregate("", (c, n) => c.ToString() + n.ToString());
var res2 = new object[] { 1, 2, 3 }.Aggregate("", (c, n) => c.ToString() + n.ToString());
var res3 = new object[] { "1", "2", "3" }.Aggregate("", (c, n) => c.ToString() + n.ToString());
Console.WriteLine("{0}: {1}", "res1", res1); //123
Console.WriteLine("{0}: {1}", "res2", res2); //123
Console.WriteLine("{0}: {1}", "res3", res3); //123
JS (http://jsfiddle.net/5k0gfaej/)
var _EnumerableImpl = Enumerable.From([]).constructor;
//expose linq directly to array type
Object.setPrototypeOf(Array.prototype, _EnumerableImpl.prototype);
var res1 = [1,"2","3"].Aggregate("", (c,n) => c.toString() + n.toString());
var res2 = [1,2,3].Aggregate("", (c,n) => c.toString() + n.toString());
var res3 = ["1","2","3"].Aggregate("", (c,n) => c.toString() + n.toString());
console.log("res1: " + res1); // 0123
console.log("res2: " + res2); // 0123
console.log("res3: " + res3); // 123
For some reason the starting object is being evaluated to zero when the first item in the array is numeric, rather than string. Check out the linked fiddles for a working example.
I've narrowed down the issue to line 193 below:
if (!result) result = Constant.getDefaultVal(typeof (value));
Since an empty string ("") will evaluate to true in this case, the default value of zero will be retrieved based on the first argument of the array (this).
Changing this line to the following should work:
if (!result && result != "") result = Constant.getDefaultVal(typeof (value));
Original full code block from 'LINQ/lib/linq.ts' starting from line 177:
public Aggregate<A, B>(alpha: (x: T|A, y: T) => T|A,
beta: (x: T|A, y?: T) => A | B = Constant.selfFn,
gamma: (x: A) => B = Constant.selfFn): B {
let zero: A;
let method: (x: A, y: T) => A;
let selector: (x: A) => B;
if (Constant.CONST_FUNCTION === typeof alpha) {
method = <(x: A, y: T) => A>alpha;
selector = <(x: A) => B>beta;
} else {
zero = alpha as any;
method = <(x: A, y: T) => A>beta;
selector = gamma;
}
let result: A = zero;
for (let value of this) {
if (!result) result = Constant.getDefaultVal(typeof (value));
result = method(result, value);
}
return selector(result);
}
I'd submit a pull request, however I'm new to source control and github. Couldn't figure out how to do it properly. Hopefully this will make it easy enough.
Gerneio
Also, here's an excerpt from stackoverflow about this:
https://stackoverflow.com/a/8693003
All
false
,0
, empty strings''
and""
,NaN
,undefined
, andnull
are always evaluated asfalse
; everything else istrue
.
So technically, if you have a starting value of ZERO (0), then you could run into this same issue, but in reverse:
["1","2","3"].Aggregate(0, (c,n) => c.toString() + n.toString()); // 123 (expected 0123)
Maybe try this instead so we test exactly for WHY we need a default value.
if ([null, undefined].indexOf(result) > -1 || (isNaN(result) && !result)) result = Constant.getDefaultVal(typeof (value));
Honestly, I think this checking should be scraped, since the user should be responsible for passing in a usable value as intended.
Gerneio
You should send a PR. As good a time to learn as any :)
Done. Hopefully I did it right!