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

davidchambers · 2017-12-17T20:59:45Z

Commit message:

def has served two roles:

currying an uncurried implementation function; and

performing type checking at run time.

At the time of this project's inception in 2015 it seemed unreasonable to require users to provide curried implementation functions. Defining curried functions manually was onerous prior to ES6:
function(x) {
  return function(y) {
    return function(z) {
      return ...;
    };
  };
}
With ES6, defining curried functions is trivial:
x => y => z => ...
The landscape has changed since 2015, and it's now reasonable to assume that users are targeting ES6. Furthermore, the advantages of Ramda-style currying over simple currying do not justify the additional complexity. There's no compelling reason for def to continue to serve two roles.

User-facing changes:

$ functions must now be applied to arguments one at a time;

def now requires a curried implementation function;

def no longer imposes an arbitrary arity limit;

def functions must now be applied to arguments one at a time; and

__ has been removed (simple currying precludes placeholders).

Internal changes:

the checkTypes option is now checked in just one place; and

def is now essentially a no-op if type checking is disabled.

This pull request embraces the f (x) (y) style proposed in sanctuary-js/sanctuary#438.

The changes to the test suite will be easier to review once #178 has been merged.

Avaq · 2017-12-19T09:41:17Z

index.js

@@ -58,23 +58,23 @@
 //.
 //. ```javascript
 //. //    env :: Array Type
-//. const env = $.env.concat([List($.Unknown)]);
+//. const env = $.env .concat ([List ($.Unknown)]);


Inconsistency: Space preceding .concat but not .env.

The space after the method name is to appease the linter. If we want to write f (x) the linter wants us to write foo.bar (baz) as well. I was experimenting with surrounding method names with spaces to make them appear more like infix operators: xs .concat (ys). What do you think?

I see a :thumbsdown: from Gabe. I certainly don't love the proposal. Let's vote!

😆 $.env.concat([List ($.Unknown)]) ✘

🎉 $.env.concat ([List ($.Unknown)]) ✔︎

❤️ $.env .concat ([List ($.Unknown)]) ✔︎

The ✔︎/✘ indicates whether we'll be able to continue to use func-call-spacing.

I like the idea of the formatting as a signal that curried functions are being used.

[1, 2, 3].concat([4, 5, 6]); // Nothing special going on here S.concat ([1, 2, 3]) ([4, 5, 6]); // FP in the house!

I can see the logic behind that, Gabe. What about unary functions, though? Math.abs(x) or Math.abs (x)? S.trim(s) or S.trim (s)?

What about require('express')(b, c)? What about fs.reduce(f, x)(y)? I think trying to categorise function calls based on underlying structure will raise more questions than it settles. I vote for the consistency of always putting a space between the function or method identifier and its application:

Math.abs (x)

require ('express') (a, b)

fs.reduce (f, x) (y)

$.env.concat ([List ($.Unknown)])

f(x)(y).z is problematic. In f (x) (y).z the .z property access appears to be bound to (y) rather than to f (x) (y). There are two reasonable solutions, each of which violates one of our current rules:

f (x) (y) .z (violates no-whitespace-before-property)

(f (x) (y)).z (violates no-extra-parens)

I prefer the latter as it is consistent with foo.bar.baz (I'm not ready to switch to foo .bar .baz).

So with this new approach, all preceding occurrences of Z.env .concat can be fixed, right?

Avaq · 2017-12-19T09:44:51Z

index.js

@@ -402,6 +393,10 @@
  var Inconsistent =
  new _Type(INCONSISTENT, '', '', always2('???'), K(false), [], {});

+  //  NoArguments :: Type
+  var NoArguments =
+  new _Type(NO_ARGUMENTS, '', '', always2('()'), K(true), [], {});


If I understand correctly what this is for, we could consider naming it Unit, right?

NoArguments and Unit represent related but distinct ideas.

Were we to define const Unit = {}; and the accompanying Type value, f () and f (Unit) would still be different expressions: one applies f to no arguments; the other applies f to one argument.

It could be confusing to name the above Type value Unit as we may one day wish to define a Type value equivalent to Haskell's (). This would logically be named Unit.

In Haskell () is a nullary tuple. I like to think that the only differences between Haskell and JavaScript, are that JavaScript forces you to pattern-match your tuples, and you can only instantiate them at call time.

// f :: Tuple a b -> c -> ... // -- alernatively written as // f :: (a, b) -> c -> ... const f = (a, b) => c => ...

When viewing it like this, Unit has no addressable value level member (only at call time).

Avaq · 2017-12-19T09:51:24Z

index.js

  //.     type; and
-  //.   - `$.Function([a, b, a])` represents the `(a, b) -> a` type.
-  function Function_(types) {
+  //.   - `$.Function ([a, b, a])` represents the `(a, b) -> a` type.


Does it still?

Yes. The behaviour of $.Function is unchanged. I believe sanctuary-def should be capable of expressing types of which we disapprove. ;)

Avaq · 2017-12-19T10:00:22Z

index.js

+  function NullaryType(name) {
+    return function(url) {
+      return function(test) {
+        function format(outer, inner) {


It looks like the format function could be shared between different applications of NullaryType (name).

Great point! I'll address this.

Avaq · 2017-12-19T10:02:12Z

index.js

+      return function(test) {
+        return function(_1) {
+          return function($1) {
+            function format(outer, inner) {


This function references $1.

Oh, I missed that.

Avaq · 2017-12-19T10:02:57Z

index.js

+          return function(_2) {
+            return function($1) {
+              return function($2) {
+                function format(outer, inner) {


This function references $1 and $2.

Oh, I missed that.

Avaq · 2017-12-19T10:03:25Z

index.js

-      return new _Type(VARIABLE, name, '', format, K(true), keys, types);
+    return function($1) {
+      return function($2) {
+        function format(outer, inner) {


Avaq · 2017-12-19T10:07:13Z

index.js

  }

-  //  fromUncheckedBinaryType :: ((Type, Type) -> Type) ->
+  //  fromUncheckedBinaryType :: (Type -> Type -> Type) ->
  //                             (Type -> Type -> Type)


Brackets for clarity?

Avaq · 2017-12-19T10:08:19Z

test/index.js

+    ([a, a, a, $.Array (a)])
+    (x => y => z => [x, y, z]);
+
+//    $26 :: a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> a -> Array a


I wanted to show that we're no longer limited to nine parameters. 😂

Avaq · 2017-12-19T10:10:05Z

test/index.js

-  });
+//    eq :: (a, b) -> Undefined !
+function eq(actual, expected) {
+  assert.strictEqual (arguments.length, eq.length);


Curious that you use the new style in the examples and the tests, but not the actual source.

I can imagine reformatting the source code too, but I'd prefer to do so in a separate pull request to avoid obfuscating the significant code changes.

Avaq · 2017-12-27T09:27:38Z

index.js

-        } else {
-          indexes.push(index);
+      return function(x) {
+        var args = slice.call(arguments);


I think we might be able to avoid calling slice.call here. It would increase performance.

Great point!

⚡

Avaq · 2017-12-27T09:33:15Z

index.js

      }
+
+      var isThunk = expType.types.$1.type.type === NO_ARGUMENTS;
+      var numArgsExpected = isThunk ? 0 : expType.keys.length - 1;


Note to self: I want to think about this approach.

My thoughts: #180

Avaq · 2017-12-27T09:36:20Z

index.js

-          vReprs.push(Z.toString(_values[idx]));
+    var wrapped = typeInfo.types[0].type === NO_ARGUMENTS ?
+      function() {
+        var args = slice.call(arguments);


Same here. We can move the slice.call part to the unhappy path.

davidchambers · 2018-03-05T02:43:14Z

I've rebased this pull request now that #184 has been merged. Please review scripts/lint and .eslint-es6.js.

Avaq

Looks good to me!

‘def’ has served two roles: - currying an uncurried implementation function; and - performing type checking at run time. At the time of this project's inception in 2015 it seemed unreasonable to require users to provide curried implementation functions. Defining curried functions manually was onerous prior to ES6: function(x) { return function(y) { return function(z) { return ...; }; }; } With ES6, defining curried functions is trivial: x => y => z => ... The landscape has changed since 2015, and it's now reasonable to assume that users are targeting ES6. Furthermore, the advantages of Ramda-style currying over simple currying do not justify the additional complexity. There's no compelling reason for ‘def’ to continue to serve two roles. User-facing changes: - ‘$’ functions must now be applied to arguments one at a time; - ‘def’ now requires a curried implementation function; - ‘def’ no longer imposes an arbitrary arity limit; - ‘def’ functions must now be applied to arguments one at a time; and - ‘__’ has been removed (simple currying precludes placeholders). Internal changes: - the ‘checkTypes’ option is now checked in just one place; and - ‘def’ is now essentially a no-op if type checking is disabled.

This was referenced Dec 17, 2017

readable code without Ramda-style currying sanctuary-js/sanctuary#438

Closed

lint: detect incorrect indentation within createSanctuary sanctuary-js/sanctuary#467

Closed

Avaq reviewed Dec 19, 2017

View reviewed changes

davidchambers force-pushed the davidchambers/simple-currying branch from 2b72b37 to e16c7b0 Compare December 26, 2017 18:41

Avaq reviewed Dec 27, 2017

View reviewed changes

Avaq mentioned this pull request Dec 28, 2017

cyclical placeholders #73

Closed

davidchambers force-pushed the davidchambers/simple-currying branch 2 times, most recently from d960c05 to 9997c9c Compare March 5, 2018 02:40

Avaq approved these changes Mar 6, 2018

View reviewed changes

davidchambers force-pushed the davidchambers/simple-currying branch 4 times, most recently from 2e6779f to 51ff546 Compare March 31, 2018 11:43

davidchambers force-pushed the davidchambers/simple-currying branch from 51ff546 to 48529ee Compare April 1, 2018 17:29

davidchambers merged commit 27c6ca3 into master Apr 1, 2018

davidchambers deleted the davidchambers/simple-currying branch April 1, 2018 17:39

evgenykochetkov mentioned this pull request Oct 5, 2018

Sanctuary >0.15.0 compatibility and general improvements xodio/hm-def#8

Merged

davidchambers mentioned this pull request Mar 28, 2019

documentation: remove vestigial reference to ‘concatS’ #242

Merged

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

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

Conversation

davidchambers commented Dec 17, 2017 • edited Loading

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 Dec 20, 2017 • edited Loading

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 Dec 20, 2017 • edited Loading

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

davidchambers commented Mar 5, 2018

Avaq left a comment

Choose a reason for hiding this comment

davidchambers commented Dec 17, 2017 •

edited

Loading

Avaq Dec 20, 2017 •

edited

Loading

Avaq Dec 20, 2017 •

edited

Loading