Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Argument against this proposal #39

Open
xixixao opened this issue May 23, 2020 · 11 comments
Open

Argument against this proposal #39

xixixao opened this issue May 23, 2020 · 11 comments

Comments

@xixixao
Copy link

xixixao commented May 23, 2020

I am trying to revive pipeline proposal and have it get going with Hack-style only version. See tc39/proposal-pipeline-operator#167 (comment)

I'd like to add for your consideration why I think we should not be moving forward with this proposal - not any of its particular variants, but in general (if we decide to move forward with any Hack-style pipeline supporting proposal). This expands on the point 5 in the comment linked above.

What are Pipelines?

First, let me explain the point of the pipeline operator: It allows you to skip naming things. Every pipeline can be rewritten:

return foo |> bar(#) |> baz(#);

like this:

const someVeryGoodName = bar(foo);
return bar(someVeryGoodName);

Now if that's the case, you might be wondering what's my gripe with this proposal. What's the difference, isn't this proposal also allowing you to skip naming things?

It is! But with one major, major difference.

In the case of the pipeline, you can see what you're not naming.

I can see foo and bar(#) in the example above, so I have some idea of what bar(foo) is.

A more realistic example from Hack:

return $test_users_not_sorted
  |> Vec\unique_by($$, $test_user ==> $test_user['user']['id'])
  |> Vec\sort_by($$, $test_user ==> $test_user['user']['name']);

It doesn't help this code to add variables unique_test_users_not_sorted and unique_test_users_sorted. I can see what's going on just as well from the pipeline code.

Partial Application

Now let's look at this proposal:

const fooBar = foo(1, ?);

What is ?? I have no idea. I'll have to inspect foo to find out, or fooBar needs to be superbly named. What's the alternative? Functions!

const fooBar = n => foo(1, n)

Even a super undescriptive name like n gives me more information (the argument is probably a number).

More examples directly from readme:

const log = console.log({ toString() { return `[${new Date()}]` } }, ?);

I have no idea what the second arg is intended to do.

button.addEventListener("click", this.onClick(?));

I have no idea what is being passed to onClick (if I pretend I don't know what addEventListener does - but even knowing it took me a second to wrap my head around this). Now I try to remember what coding was like when I got started, and I really would not want to be learning from such code (or wishing it upon anyone else).

There are other arguments against this proposal, like its weirdly restricted nature to only some forms of expressions (why not others? we could argue for years).

Just like in the discussion on pipelines I will advise: Swallow the bullet and add those 5 characters. If you're not writing an entirely throw-away code you will want to have a good name for that argument. This proposal is way too complicated for the little benefit of saving those 5 characters in the few special cases it applies.

In summary, this proposal got started to support pipelines, pipelines don't need it if we go with Hack-style only, and in that case we should drop this proposal.

cc @codehag @ducaale @aadamsx who are advocating the other direction in #36

@ducaale
Copy link

ducaale commented May 23, 2020

In the case of the pipeline, you can see what you're not naming.

I can see foo and bar(#) in the example above, so I have some idea of what bar(foo) is.

Wouldn't this very proposal let you do the same thing?

return foo |> bar(?) |> baz(?);

What is ?? I have no idea. I'll have to inspect foo to find out, or fooBar needs to be superbly named. What's the alternative? Functions!

Just like in the discussion on pipelines I will advise: Swallow the bullet and add those 5 characters. If you're not writing an entirely throw-away code you will want to have a good name for that argument. This proposal is way too complicated for the little benefit of saving those 5 characters in the few special cases it applies.

In some functional languages, currying happens by default. This proposal is just trying to make that more natural to write.

An example of a curry-by-default language

let add a b = a + b
let increment = add 1
printfn "%d" (increment 8)

Simulating the same behaviour with this proposal

const add = (a, b) => a + b
const increment = add(1, ?);
console.log(increment(8))

const log = console.log({ toString() { return [${new Date()}] } }, ?);
I have no idea what the second arg is intended to do.

I think that example is broken. It is most likely from an earlier version of the proposal when it was trying to support partially applied templates #34. I think the intention was for it to be something like const log = console.log(`[${new Date()}] ${?}`)

@noppa
Copy link

noppa commented May 23, 2020

There are cases where being explicit with the helper function argument names is warranted and there are cases where I think they bring little to no value.

const parsedValues = userInputs.map(n => parseInt(n))
const parsedValues = userInputs.map(parseInt(?))

I don't think the latter of those is any less readable than the former. On the contrary, I find the extra n unnecessary noise that actually hurts readability a bit.


@ducaale

const log = console.log({ toString() { return [${new Date()}] } }, ?);

That example just creates a log function whose first argument is set to always be

{ toString() { return `[${new Date()}]` } }

and second argument is passed in later when log is called. I think the toString object wrapper thingy is there because just doing

const log = console.log(`[${new Date()}]`, ?)

would evaluate that eagerly and all log calls would have the same timestamp. They are relying on the console.log implementation to call the object's toString method so each log gets a new timestamp. At least currently Node, Chrome and Ff don't seem to call it, though, so I guess in that sense it is broken.

@hax
Copy link
Member

hax commented May 24, 2020

Personally I liked partial application, but I think the example of log is a very interesting example to let us see the other side:

const log = console.log({ toString() { return [${new Date()}] } }, ?);

vs

const log = x => console.log(`[${new Date()}]`, x)

Even the partial application version worked as expect, I think the arrow function version is much easy to write, read and understand.


So maybe we'd better think the whole things again, do the proposals (not only F# style + partial application, but also Hack/smart style) really add enough benefit?

@lazarljubenovic
Copy link

Whenever there's a new proposal, comments start popping where rationales such as this creep in:

What is ?? I have no idea.

Well, you'd have an idea if you'd learn it as part of the language. I mean, look at this code.

const a = x ? 1 : 2

What is ?? What is :? I have no idea.

Heck, even =. How is it equal? Ha, it's not! But how do I know = means "assignment" and not "equals"? Who the heck thought of ==? What's ==? What's ===? Well, I learned it.

I have no idea what is being passed to onClick (if I pretend I don't know what addEventListener does - but even knowing it took me a second to wrap my head around this).

Of course it took you a second. It takes everyone a second because this is the first time we're looking at this new syntax. It used to take me more than a second to understand what ({ a: b }) => ({ b }) does. It took me forever to wrap my head round currying as well. fn(a)(b)(c) what the heck is this?! Ooh you return a function. And then you call it again, etc.

It took me a second to realize exactly what a ?? b does. I was like "duh just write a != null ? a : b or a || b if it works, what's the big deal? After using it a couple of times it's become obvious to me.


I understand that discussing the syntax and pointing out footguns is a good thing, but the whole rationale here is basically "I'd rather write an arrow function because I'm used to it and there are cases where it makes code easier to understand".

For example, I have no idea what Hack is and seeing this

return $test_users_not_sorted
  |> Vec\unique_by($$, $test_user ==> $test_user['user']['id'])
  |> Vec\sort_by($$, $test_user ==> $test_user['user']['name']);

I'm so confused. What's ==>? What's $$? What's \? Nothing is immediately obvious either.

Swallow the bullet and add those 5 characters. If you're not writing an entirely throw-away code you will want to have a good name for that argument.

Funny to say that since you use anonymous functions in the code. Why not swallow the bullet and write a good name for a function?

arr.map(x => x + 1)
arr.map(function increment (x) { return x + 1 })

Now, back to

const fooBar = foo(1, ?);

What's ?? Well, it's the argument that's gonna get filled in when fooBar is called with a single argument. And if you don't like using such syntax in this instance, well... You couldn't have said it better:

What's the alternative? Functions!

@xixixao
Copy link
Author

xixixao commented May 26, 2020

@lazarljubenovic thanks for your thoughtful comment which clearly showcases that I have not explained my argument well enough. I'll try to elaborate and perhaps it'll help get the point across.

My point isn't that ? is a new peace of syntax I don't understand. In x |> foo(#), even if I knew pipes (|>), the topic reference (#) would be a foreign piece of syntax to which such an argument would apply just as well.

That is not my point.

My point is that in an expression:

a |> b(#)

I know what is being passed to b, it's a. I know because it's right there.

In an expression

b(?)

I don't know what is or can be passed to b.

This is why it's beneficial, for code readability, to have the expression written as

someName => b(someName)

because this gives me a better chance to understand the code.

So I hope this clarifies my point.

Now to your last point, which boils down "You don't have to use the syntax when it's added", this is not really a valid argument for a language syntax discussion. One could use this argument for any backwards-compatible piece of syntax. If we followed this logic we would end up with a language with an infinite number of features. Besides the cost associated with each feature (complications to compilers and interpreters, and to tooling), the most important counter argument is that I do not choose how others will write code. So the possibility of writing a piece of code in less confusing syntax is not an argument for adding that confusing piece of syntax.

@ducaale
Copy link

ducaale commented May 26, 2020

@xixixao Instead of comparing a |> b(#) and b(?) / n => b(n), shouldn't the comparison be between a |> b(#) and a |> b(?) / a |> n => b(n)?

@xixixao
Copy link
Author

xixixao commented May 27, 2020

@ducaale I am specifically calling out that this proposal should not move forward if the Hack-style pipeline proposal (or for that matter the smart pipeline proposal) moves forward.

So my only concern is the non-pipeline use of this new syntax.

@aadamsx
Copy link

aadamsx commented May 27, 2020

@xixixao Partial Application is a good addition and can stand on its own. But if BOTH pipeline styles move forward, then partial application becomes more important: tc39/proposal-pipeline-operator#167 (comment)

@treybrisbane
Copy link

@xixixao My interpretation of this argument is that

  1. You feel that const bar = foo(x, ?); makes what you can pass to bar less clear than const bar = y => foo(x, y);
  2. You feel that it should be mandatory to explicitly name the parameters when defining a function
  3. You feel that point-free style should be avoided

Is that interpretation correct?

If so, I strongly disagree.

On point 1.

I don't see this being a real issue in practice.

When a function is in-scope, it's because it's either:

  • Locally defined (either in the current scope, or in an enclosing scope), or
  • Externally defined (either imported or passed in as an argument)

If bar is locally defined, it may in some circumstances be clearer if it were defined with const bar = y => foo(x, y);, but only if the parameter name (i.e. y) accurately reflects what bar's inner function (i.e. foo) can accept. If that parameter name is inaccurate or misleading, then it may imply that bar accepts values that foo cannot, resulting in runtime errors that are potentially quite difficult to debug.

For example:
multiplyInt.js

export const multiplyInt = (x, y) => {
  if (typeof x !== 'bigint' || typeof y !== 'bigint') {
    throw new TypeError('multiplyInt only accepts integers');
  }

  return x * y;
};

main.js

import { multiplyInt } from './multiplyInt.js';

// `double` is ambiguous; `n` implies the argument could be a `number`
const double = n => multiplyInt(2, n);

console.log(double(4));
// TypeError: multiplyInt only accepts integers

In this case, I'd argue that the partial application proposal would remove the ambiguity:

// `double` is clear; it accepts only what `multiplyInt` accepts as its second argument
const double = multiplyInt(2, ?);

On the other hand, if bar is externally defined, then all bets are off; it's not possible to know what it accepts without checking its implementation.

For example:
handleEach.js

export const handleEach = (handler, iterable) => {
  for (const item of iterable) {
    // No way to know what `handler` accepts here (or even if its a function, really) without looking at the call sites of `handleEach`
    handler(item);
  }
};

Overall, the concern of const bar = foo(x, ?); being less clear than const bar = y => foo(x, y); only ever applies if bar is locally defined, and even then it's debatable.

On point 2.

I don't agree that it should be mandatory to explicitly name the parameters when defining a function.

Firstly, as I alluded to above, when you define a function bar with const bar = y => foo(x, y);, the name you choose for bar's parameter is actually an assumption about what foo accepts. Such an assumption may be valid at the time bar is added, but if foo later changes, that assumption could be inadvertently invalidated. Again, this may result in anything from immediate crashes to cascading errors propagating through a system.
In contrast, when you define a function bar with const bar = foo(x, ?);, the only assumption being made about foo is its arity (which, incidently is also the case with the arrow function style).

In other words, the partial application proposal allows you to make less assumptions when defining certain classes of functions.

Secondly, the declarations const bar = foo(x, ?); and const bar = y => foo(x, y); actually differ slightly in intent.

const bar = y => foo(x, y); declares a function bar that takes a value y and passes it (along with x) to a function foo.
That is, the intent seems to be to declare a function that takes a value and passes it to another function.

const bar = foo(x, ?);, in contrast, declares a function bar that specifically takes the second argument of a function foo and passes it (along with x) to that function.
That is, the intent seems to be to specifically declare a partial application of foo.

In other words, the omission of the parameter name can itself be an important piece of information.

Thirdly, it's already possible to define functions without naming the parameters.

For example:
main.js

const bar = foo.bind(null, x);

This is already something that many developers (including myself) do with varying degrees of regularity. There are notable downsides to this approach, however:

  • It's verbose, reducing code clarity with every use
  • It's actually less clear than the partial application proposal in terms of what bar can accept as input
    • Unclear what the arity of bar is without checking foo's implementation
  • TypeScript has trouble correctly typing this function in all cases

In other words, we can already define functions in this manner, however the means of doing so results in less clarity and less usability than the partial application proposal.

Finally, requiring explicit parameter names when defining functions carries the assumption that these names are the only (or even the primary) means by which developers describe function contracts.

With the emergence of static typing in the JS ecosystem, this is no longer the case. TypeScript for instance allows for the definition of functions with highly specific, strongly enforced contracts that go well beyond simple naming. In a statically typed world, parameter names don't matter nearly as much for either code clarity or code correctness.

For example:
main.ts

// `multiplyInt` is of type `(x: bigint, y: bigint) => bigint`
const multiplyInt = (x: bigint, y: bigint): bigint => x * y;

// `double` would be inferred as type `(y: bigint) => bigint`
const double = multiplyInt(2, ?);

In other words, the lack of explicit parameter names is even less likely to be a problem in a statically typed world, allowing TypeScript users to get the benefits of the partial application proposal with even less potential downsides.

On point 3.

For what it's worth, I personally prefer point-free style in my functional code, so I disagree with avoiding it.

Frankly, preferences around things like point-free style are almost entirely subjective, so debating them is questionable IMO. What I will say though is that you personally not liking or not seeing value in point-free style doesn't mean that others feel the same. Nor does it mean that a partial application operator wouldn't be a useful or "good" addition to the language.

There's something to be said for enabling alternative programming styles and increasing language diversity, even if you personally don't have an interest in those particular areas.

@CrossEye
Copy link

@treybrisbane:

While I agree entirely with your arguments (in fact I maintain a library dedicated to FP in JS and remain a big fan of point-free programming), I would like to note that there is also something important to be said against "enabling alternative programming styles and increasing language diversity."

That is the matter of adding complexity.

There is a tricky balance to maintain between adding useful features and bloating a language with too many competing ideas. I come down firmly on the side of this being a useful feature, in this guise or some alternative. But it does add conceptual weight to the language and makes it some degree more difficult to add future features. We should always keep this in mind alongside our considerations of utility.

@Pokute
Copy link

Pokute commented Jun 22, 2020

@CrossEye, when the context of this issue is about not needing partial application if smart/hack pipelines are added, then it is true, there will be one additional syntax.

On the other hand, if you consider that with both styles of pipelines, every programmer needs to learn practically the same concept weight anyway, the syntax cost by itself is negligible. There will actually be confusion if users see the partial application pattern and find out that it isn't usable outside pipelines.

Pipelines without any kind of partial application capability would be unwieldy for all but few coding styles, strongly favouring them. Prematurely killing off partial application proposal would deal a severe blow to minimal & F# style pipelines. Just like getting partial application to the spec first would favour both minimal & F# style pipelines.

@xixixao, attacking one part of two complementary proposals by claiming that another, competing proposal, if implemented, would take care of it, is in my opinion an unfair move. The fact that F# pipelines and partial application don't technically require each other means that passing it through TC39 as a single proposal would be unlikely.

I believe that either success of Hack/Smart pipelines or failure of partial application proposal likely results in the failure of both sides of the F#+partial application combo. I'm ready to see this thread again when something passes TC39 stage 3 or 4. Just a chance of it happening shouldn't be enough!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants