Add S.sum and S.product #192
Conversation
| function(a, b) { return a + b; }); | ||
| S.add = add; | ||
|
|
||
| //# sum :: [FiniteNumber] -> FiniteNumber |
There was a problem hiding this comment.
Should the type be as follows?
sum :: Foldable f => f FiniteNumber -> FiniteNumber| def('product', | ||
| {}, | ||
| [$.Array($.FiniteNumber), $.FiniteNumber], | ||
| reduce(S.mult, 1)); |
svozza
force-pushed
the
sum-product
branch
2 times, most recently
from
23e868c to
b08fd83
Compare
|
Just realised I forgot to do |
svozza
force-pushed
the
sum-product
branch
2 times, most recently
from
1e7cb9e to
2cb07b1
Compare
| {}, | ||
| [$.Array($.FiniteNumber), $.FiniteNumber], | ||
| reduce(function(a, b) { return a + b; }, 0)); | ||
|
|
There was a problem hiding this comment.
It looks as though each of these functions can now be moved alongside its description. sum is referenced in the definition of meld, but will have been defined by the time the body of that function is executed.
There was a problem hiding this comment.
No, the tests fail if I move them and do something like
var sum = S.sum =There was a problem hiding this comment.
Ah, because meld is applied in the definition of mapMaybe. Let's change that definition from meld([R.map, justs]) to something along these lines:
function(f, xs) { return justs(R.map(f, xs)); }
|
Let's also test these: S.sum(S.Nothing())
S.sum(S.Just(42))
S.sum(S.Left('XXX'))
S.sum(S.Right(42))The last two won't work until we resolve #119, so we could put these assertions in |
|
I've updated it to use the Foldable type constraint. I'm not happy with the error messages for the |
I suggest replacing Complaining about the return value isn't ideal, but it's the best we can do until sanctuary-def permits us to express |
|
Yeah, it's not great but I guess there's nothing we can do. |
| return foldable.reduce(f, initial); | ||
| } | ||
| }); | ||
| S.reduce = reduce; |
There was a problem hiding this comment.
We can revert this change, I believe.
| @@ -2378,7 +2379,8 @@ | |||
| //. > S.reduce((xs, x) => [x].concat(xs), [], [1, 2, 3, 4, 5]) | |||
| //. [5, 4, 3, 2, 1] | |||
| //. ``` | |||
| S.reduce = | |||
| // reduce :: Foldable f => (a -> b -> a) -> a -> f b -> a | |||
There was a problem hiding this comment.
This line is now redundant.
|
Anything else left on this? |
This looks great, Stefano. This has motivated me to resolve #119. I plan to do so tomorrow. Once that pull request is merged, you'll be able to rebase this one and remove the |
| eq(S.product(S.Just(42)), 42); | ||
| }); | ||
|
|
||
| it.skip('can be applied to Eithers', function() { |
There was a problem hiding this comment.
You can remove the .skip now thanks to #204. :)
svozza
force-pushed
the
sum-product
branch
2 times, most recently
from
9982725 to
6345074
Compare
| def('sum', | ||
| {f: [Foldable]}, | ||
| [f, $.FiniteNumber], | ||
| reduce_(function(a, b) { return a + b; }, 0)); |
There was a problem hiding this comment.
Might this be better?
reduce(function(a) { return function(b) { return a + b; }; }, 0)reduce_ will curry the provided function, but we could remove a step by defining a curried function.
We needn't worry about this, though. Once we can express f FiniteNumber in the type system we'll be able to use S.add in this definition without references to that function appearing in error messages. :)
|
🌳 Lovely patch, Stefano! |
I had to move a few functions to the top of the file so that we could replace the Ramda versions used throughout the code.