abandon Ramda-style currying in favour of simple currying #520

davidchambers · 2018-04-04T13:08:07Z

Closes #438

https://github.com/sanctuary-js/sanctuary-def/releases/tag/v0.15.0

Avaq · 2018-04-05T09:48:53Z

That is a pretty daunting diff.

Most seem like stylistic changes of sorts, eg a, b -> a) (b. Could you point me to the significant changes, if any? Or otherwise if you'd really like me to take a good look at it, we could arrange a call of sorts so you can guide me through this.

davidchambers · 2018-04-05T11:19:28Z

That is a pretty daunting diff.

I agree. I am reviewing the changes myself and don't know where to focus my attention. I'll post comments on some sections which strike me as potentially controversial. :)

davidchambers · 2018-04-05T11:21:26Z

eslint/es6.js

+      },
+    },
+  ],
+};


This file allows us to verify that doctests and code snippets follow our desired formatting rules.

davidchambers · 2018-04-05T11:23:39Z

eslint/rules/indent.js

+  'CallExpression > ArrowFunctionExpression ArrowFunctionExpression > *',
+  'CallExpression > FunctionExpression > BlockStatement'
+);
+module.exports = indent;


ESLint does not like this indentation:

f (x) (y) (z)

Rather than disable the indent rule entirely, we can add exceptions for problematic cases.

davidchambers · 2018-04-05T11:35:09Z

scripts/lint

+cp README.md README.md.temp
+rewrite <README.md.temp >README.md
+
+node_modules/.bin/eslint --config eslint/es6.js -- README.md eslint


This script enables npm run lint to run eslint on files that would not otherwise be linted.

Linting the readme involves a number of steps:

Save a copy of the file as it exists.

Regenerate the readme (the VERSION and PREVIOUS_VERSION values are unimportant).

Use rewrite to remove leading > and . characters, parenthesize object literals which would otherwise be parsed as blocks, and insert a semicolon at the end of each input and output line.

Run eslint.

Restore the readme to its original state (via an EXIT trap).

sanctuary-js/sanctuary-scripts#13

davidchambers · 2018-04-05T11:36:49Z

test/internal/eq.js

+    assert.strictEqual (arguments.length, eq$1.length);
+    assert.strictEqual (toString (actual), toString (expected));
+    assert.strictEqual (equals (actual) (expected), true);
+  };
 };


The internal eq function is now curried.

davidchambers · 2018-04-05T11:42:15Z

test/internal/throws.js

+  return function throws$1(expected) {
+    assert.strictEqual (arguments.length, throws$1.length);
+    assert.throws (thunk, equals (expected));
+  };
 };


The internal throws function is now curried, and now takes a single value representing the expected error rather than a type representative and an error message.

Before:

throws(thunk, TypeError, 'Invalid value');

After:

throws (thunk) (new TypeError ('InvalidValue'));

davidchambers · 2018-04-05T11:43:24Z

test/internal/List.js

+  return function Cons$1(tail) {
+    eq (arguments.length) (Cons$1.length);
+    return new _List ('Cons', head, tail);
+  };
 };


Cons is now curried.

davidchambers · 2018-04-05T11:44:40Z

index.js

+    return function(y) {
+      return $.Function ([x, y]);
+    };
+  }


Fn is now curried.

davidchambers · 2018-04-05T11:46:51Z

index.js

+    (readmeUrl ('EitherType'))
+    (typeEq (eitherTypeIdent))
+    (either (of (Array)) (K ([])))
+    (either (K ([])) (of (Array)));


Now that functions such as K are curried (and hoisted) we're able to use them throughout the file. :)

davidchambers · 2018-04-05T11:54:47Z

index.js

  //.
  //. Returns `true` [iff][] the *second* argument is less than the first
-  //. according to [`Z.lt`][]. The arguments must be provided one at a time.
+  //. according to [`Z.lt`][].


This is one of the important changes listed in #438 (comment). Now that all functions are “simply” curried there's no reason for this function's type to be [a, $.Predicate (a)] rather than [a, a, $.Boolean]. :)

davidchambers · 2018-04-05T11:58:16Z

index.js

+    return function(filterable) {
+      return Z.filter (pred, filterable);
+    };
+  }


We are defining filter rather than setting impl to curry2 (Z.filter) in order to partially apply the function in other definitions.

davidchambers · 2018-04-05T12:02:03Z

index.js

+      };
+      function step(next, done, x) {
+        return Z.map (either (next) (done), f (x));
+      }


I decided to rely on function hoisting here to avoid interrupting the function > return > > return staircase.

davidchambers · 2018-04-05T12:03:49Z

index.js

+        return function(x) {
+          return function(y) {
+            return function(z) {
+              return f (v, w, x, y, z);


For some reason this makes me happy. :D

I was typing a similar comment to curry:

So blissfully transparent! :D

...but then I scrolled down and saw curry2, which made me happier, so I removed the comment to move it over there. But then I scrolled down further to find curry3. Woah, how cool is this going to get? I scrolled all the way to curry5 and found your comment. :)

davidchambers · 2018-04-05T12:09:21Z

index.js

@@ -1711,7 +1773,7 @@
  //. > S.Nothing.isNothing
  //. true
  //.
-  //. > S.Just(42).isNothing
+  //. > (S.Just (42)).isNothing


This is a situation in which unnecessary parens are justified: isNothing in S.Just (42).isNothing is tightly bound, visually, to 42.

Fortunately, such property access is rare in code written with Sanctuary (one could use S.isNothing in this case).

davidchambers · 2018-04-05T12:20:45Z

index.js

  //. 'Just([1, 2, 3])'
  //. ```
  Maybe.prototype.toString = function() {
-    return this.isJust ? 'Just(' + Z.toString(this.value) + ')' : 'Nothing';
+    return this.isJust ? 'Just(' + Z.toString (this.value) + ')' : 'Nothing';


At some point I would like to switch from 'Just(42)' to 'Just (42)', but doing so now would lead to inconsistencies such as 'Just (new Date(0))'. This order would be optimal, I believe:

Create sanctuary-show, copying the Z.toString implementation but using f (x) style.

Update sanctuary-maybe to depend on sanctuary-show, and replace Maybe#toString with Maybe#@@show which uses f (x) style.

Update this project to depend on sanctuary-maybe and sanctuary-show, and replace S.toString with S.show.

davidchambers · 2018-04-05T12:22:42Z

index.js

+  function maybe(x) {
+    return function(f) {
+      return function(maybe) {
+        return maybe.isJust ? f (maybe.value) : x;


It feels right to define fromMaybe in terms of maybe rather than the other way around.

davidchambers · 2018-04-05T12:23:16Z

index.js

-    types: [Fn(a, $Maybe(b)), f(a), f(b)],
-    impl: mapMaybe
+    types: [Fn (a) ($Maybe (b)), f (a), f (b)],
+    impl: blackbird (justs) (map)


Using blackbird may be a step too far. ;)

I would think so. Especially if used sparsely and considering that the definition of blackbird itself is also somewhat mind-bending.

Now that I've figured out what's going on, I also see the appeal though ;)

I've removed blackbird. The implementation is now B (B (justs)) (map).

The implementations of encase2 and encase3 sell me on the idea:

B (B (B (eitherToMaybe))) (encaseEither2 (I)) B (B (B (B (eitherToMaybe)))) (encaseEither3 (I))

Beautiful!

On several occasions I've defended map (map (map (map (nbsp)))). I feel a similar way about B (B (B (B (eitherToMaybe)))). It looks strange at first, but once one learns how to interpret the nesting it's much clearer than the equivalent nested lambdas.

davidchambers · 2018-04-05T12:25:53Z

index.js

-    types: [$.Predicate(a), a, $.Boolean],
-    impl: complement
+    types: [$.Predicate (a), a, $.Boolean],
+    impl: compose (not)


Curried compose is very useful!

I have been saying the same thing! This is one of the reasons why Ramda's compose is so crippled, but you don't really realise until you try it.

davidchambers · 2018-04-05T12:29:26Z

index.js

    impl: find
  };

-  //# unfoldr :: (b -> Maybe (Pair a b)) -> b -> Array a
+  //# unfoldr :: (b -> Maybe (Array2 a b)) -> b -> Array a


We're now using $.Array2, which replaced $.Pair.

Avaq

I would be interested to find out how these changes affect performance. I would imagine JavaScript function context creation time has increased quite a lot.

dakom · 2018-07-06T08:33:39Z

I would be interested to find out how these changes affect performance. I would imagine JavaScript function context creation time has increased quite a lot.

That would be fascinating to see benchmarks, ideally of actual projects (not just adding a bunch of numbers)

davidchambers requested a review from Avaq April 4, 2018 13:08

davidchambers commented Apr 5, 2018

View reviewed changes

index.js

return function(y) {

return $.Function ([x, y]);

};

}

Copy link

Member Author

davidchambers Apr 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fn is now curried.

davidchambers commented Apr 5, 2018

View reviewed changes

Avaq approved these changes Apr 5, 2018

View reviewed changes

abandon Ramda-style currying in favour of simple currying

359240f

davidchambers force-pushed the davidchambers/simple-currying branch from 4d8d321 to 359240f Compare April 6, 2018 15:54

davidchambers merged commit a3fd05a into master Apr 6, 2018

davidchambers deleted the davidchambers/simple-currying branch April 6, 2018 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

abandon Ramda-style currying in favour of simple currying #520

abandon Ramda-style currying in favour of simple currying #520

davidchambers commented Apr 4, 2018

Avaq commented Apr 5, 2018 •

edited

Loading

davidchambers commented Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 29, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

Avaq Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

Avaq Apr 5, 2018

Avaq Apr 5, 2018

davidchambers Apr 5, 2018

davidchambers Apr 5, 2018

Avaq Apr 5, 2018

davidchambers Apr 5, 2018

Avaq left a comment

dakom commented Jul 6, 2018

abandon Ramda-style currying in favour of simple currying #520

abandon Ramda-style currying in favour of simple currying #520

Conversation

davidchambers commented Apr 4, 2018

Avaq commented Apr 5, 2018 • edited Loading

davidchambers commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Avaq left a comment

Choose a reason for hiding this comment

dakom commented Jul 6, 2018

Avaq commented Apr 5, 2018 •

edited

Loading