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

Added value of hack-style proposal against temporary variables #200

Closed
loreanvictor opened this issue Sep 10, 2021 · 178 comments
Closed

Added value of hack-style proposal against temporary variables #200

loreanvictor opened this issue Sep 10, 2021 · 178 comments
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@loreanvictor
Copy link
Contributor

loreanvictor commented Sep 10, 2021

TLDR

The current argument for hack-style proposal against temporary variables is:

  • Picking unique and meaningful names for the intermediate values is hard
  • Unique and meaningful names for intermediate values make the code wordy and too tedious to write / read

In the hack-style proposal, we still have a variable representing the intermediate values. However, we have overcome these two issues simply by giving it a consistent, short and meaningless name, i.e. ^.

This means the argument currently mentioned against temporary variables IS NOT resolved by the pipeline operator itself, it is simply resolved by picking a consistent, short and meaningless placeholder token. In other words, we DO NOT need a new operator (with new syntax support) for resolving these issues, we could simply go the temporary, single-use variables route and just drop the need to pick unique and meaningful names for them, and basically achieve the same thing.

The main other added value that I can think of for hack-style pipelines vs temporary variables is the fact that the whole pipeline is a singular expression resolving to a value, which is not true for a sequence of assignment statements. It is notable that the real gain here is also only in short and simple pipelines.

Explanation

This is the example code mentioned against temporary variables:

const envarKeys = Object.keys(envars)
const envarPairs = envarKeys.map(envar =>
  `${envar}=${envars[envar]}`);
const envarString = envarPairs.join(' ');
const consoleText = `$ ${envarString}`;
const coloredConsoleText = chalk.dim(consoleText, 'node', args.join(' '));
console.log(coloredConsoleText);

Its issues are resolved by the pipeline operator like this:

envars
|> Object.keys(^)
|> ^.map(envar =>
  `${envar}=${envars[envar]}`)
|> ^.join(' ')
|> `$ ${^}`
|> chalk.dim(^, 'node', args.join(' '))
|> console.log(^);

However, we do not need the operator itself for gaining the benefits here. If we were to agree on using meaningless variable names for intermediate values in such chains, then we could simply rewrite the original code like the following:

let _= envvars
_= Object.keys(_)
_= _.map(envar =>
  `${envar}=${envars[envar]}`)
_= _.join(' ')
_= `$ ${_}`
_= chalk.dim(_, 'node', args.join(' '))
_= console.log(_)

Note that:

  • The chain body consumes exactly the same number of symbols (and characters)
  • The only difference is the initial let _= overhead, which seems ignorable (since it doesn't scale with the chain size / complexity)
  • Readability of _= vs |> is debatable. While |> seems visually more close to the concept of pipelines, _= benefits from familiarity (people already know what it means and don't need to learn a new concept)
  • Arguably, _ is a better placeholder character than ^, both visually, and because ^ is already an operator in the language as well (though one that is not that commonly used)

But in real life, we tend to use meaningful names for variables

With the hack-style proposal, we are already conceding on that front, as the placeholder variable will be a meaningless variable name. Besides some visual preference and some potential confusion with existing operators, there isn't much difference between using ^ to represent the intermediate value and using _. This means all the issues that would rise from NOT using meaningful names for intermediate variables would also apply to current hack-style proposal, and this cannot be used as an argument for the hack-style proposal against temporary variables.

Statements vs Expressions

One argument that could be made in favor of hack-style pipelines vs temporary variables is that a pipeline expression is an expression, resolving to a singular value. This means you could do things like this:

const x = a |> b(^) |> c(^)
const f = x => x |> h(^) |> g(^)
h(a |> b(^) |> c(^))
(a |> b(^) |> c(^)).someMethod()

The last case can easily be re-written as the following:

a |> b(^) |> c(^) |> ^.someMethod()

The third case is arguably bad practice, as making the flow of execution consistent with reading direction is one of the main purposes of pipelines to begin with, which is violated here. It can also be easily re-written like this:

a |> b(^) |> c(^) |> h(^)

As for the second case, with longer pipelines that have more complex expressions. This means for a typical clean code, each step in the pipeline would occupy its own line and the overall overhead of having a function with a return statement would be minimized. A similar argument can be made for the first case, were the assignment would happen at the end of the sequence instead of at the beginning, without much overhead between the two cases.

None of these arguments hold for short pipelines with simplistic expressions, where there might be a value in writing the whole pipeline in a single line / expression. In these cases though, the overall added value of pipelining is marginalized, and it might not be as clear whether the overhead of adding new syntax to the language would be worth such marginalized gains or not.

@haltcase
Copy link

If we were to agree on using meaningless variable names for intermediate values in such chains

Emphasis mine. This is the main issue with your suggestion, as it's highly unlikely a majority of users would agree on this style vs a pipeline operator.

Much of your argument around the aesthetics of _= also depends on it being formatted as if it were a single token, but many lint rules would add a space around the assignment operator. I do agree that _ is a better placeholder, but unfortunately that ship most likely sank in the harbor (unless the committee is on board with using _ or it despite the fact they're already valid identifiers).

Lastly I'd add that while the "naming things is hard" argument might be the driving one behind Hack-style specifically, it is not the primary benefit of the pipeline operator itself. That would be its more linear and less nested ordering of function calls.

@kosich
Copy link

kosich commented Sep 10, 2021

NOTE: a similar issue was discussed previously #173 with some interesting pros and cons, though back then we compared it to both Hack-style and F#-style pipelines.

I agree with the idea that assignment sequence seriously questions added value of Hack-style pipelines in the language.

@citycide, I think the agreement needs be achieved only inside a single codebase.
And difference between _= and _ = is negligible. Plus additional linting/formatting rules can fix that if needed.
Although, to be fair, this approach is rarely to be seen in real world code bases (argument taken from aforementioned issue).

@loreanvictor
Copy link
Contributor Author

Emphasis mine. This is the main issue with your suggestion, as it's highly unlikely a majority of users would agree on this style vs a pipeline operator.

If I understand you correctly, this means people would not converge on a single variable name for this, so let's force one upon them. Sure, but that still does not require an operator. You could basically achieve that by having _ (or ^ or whatever token) to hold the value of last assignment statement (or better yet, last expression).

Much of your argument around the aesthetics of _= also depends on it being formatted as if it were a single token, but many lint rules would add a space around the assignment operator.

Isn't agreeing on (and changing) linter rules easier than introducing new syntax to the language itself?

Lastly I'd add that while the "naming things is hard" argument might be the driving one behind Hack-style specifically, it is not the primary benefit of the pipeline operator itself. That would be its more linear and less nested ordering of function calls.

But it is (according to the README of this proposal) the main benefit over temporary variables. In other words, you do get the more linear and less nested ordering of function calls with assignment sequences as well (that is without the pipeline operator).

@ljharb
Copy link
Member

ljharb commented Sep 10, 2021

It’s definitely still very different from temporary variables. Having to name each step, or having to reuse an identifier, are problems; reusing a syntactic token is not.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 10, 2021

Having to name each step

Normally you don't have to. In fact, the hack-style pipeline proposal simply chooses a name for you for resolving that issue and nothing more. We could agree on the such a token without new syntax being added to the language.

Perhaps this example would help me better clarify myself: What if we just got ^, which would be the value of last expression, without the |> operator? Wouldn't that achieve the same thing?

envvars;
Object.keys(^);
^.map(envar =>
  `${envar}=${envars[envar]}`);
^.join(' ');
`$ ${^}`;
chalk.dim(^, 'node', args.join(' '));
console.log(^);

Of course this code is harder to read, because the value of ^ changes without any apparent assignment, so ^ means different things depending on which step you are looking at, but doesn't that also hold true when you add a |> add the beginning of each line?

having to reuse an identifier

I think I am missing something here. How is re-using ^ different than re-using _ in the examples I provided?

@ljharb
Copy link
Member

ljharb commented Sep 10, 2021

It’s entirely different, because i can’t make a variable named ^. Javascript devs know what valid variable names are.

@haltcase
Copy link

These fake-pipeline reassignment chains come with their own overhead, so claiming they are somehow simpler is a dubious argument. @theScottyJam previously made good points on this topic so I will defer to what was said there already.

@loreanvictor:

But it is (according to the README of this proposal) the main benefit over temporary variables. In other words, you do get the more linear and less nested ordering of function calls with assignment sequences as well (that is without the pipeline operator).

It isn't even the first benefit listed there — there are three listed before it that you're skipping over:

  • Deep nesting is hard to read
  • Method chaining is limited
  • Pipe operators combine both worlds

@ljharb:

It’s entirely different, because i can’t make a variable named ^. Javascript devs know what valid variable names are.

Agreed, and so do engines, compilers, and linters, some of which could raise errors or similarly guide devs to use a feature properly. Reusing a variable is more complex and error prone in comparison.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 10, 2021

It isn't even the first benefit listed there — there are three listed before it that you're skipping over

@citycide I am not. I rather feel I am unable to convey properly what I am talking about. Let me try one more time:

  1. The argument is that pipelines are good, because they provide convenience of method chaining and applicability of expression nesting (as mentioned here)
  2. Temporary variables can also be used to achieve the convenience of method chaining and applicability of expression nesting (this point is raised here). This is NOT questioning these benefits, it is just mentioning that there already are methods in the language to achieve those benefits.
  3. The counter-argument is that picking names is hard and it makes the code too wordy (this section, third paragraph). This is an answer to the question Why shouldn't we just use temporary variables instead of pipeline operators? In this discussion, the primary benefits of pipelines become irrelevant, since we are discussing another alternative that DOES provide the same benefits.
  4. My point is that syntactically hack-pipeline is (or rather, can be) pretty close to use of temporary variables, so the arguments of "wordiness" and "tediousness" mentioned in the aforementioned section are moot (the same number of symbols and tokens). On the proposal, as it stands, I do not see any other benefit listed for pipelines vs temporary variables. Again, in this discussion, the primary benefits of pipelines that you are mentioning do not play a role, because temporary variables do provide them as well.

To put this schematically: we want to achieve A and B, X and Y are two ways of achieving them. The question is now whether we should pick X or Y. Someone mentions that because of C we should pick X. I am pointing out that C is moot in the way we are going about X, are there any other reasons to favor X over Y? Now saying "but you are forgetting about A and B" does not contribute to the discussion, because both X and Y achieve A and B and pointing them out does not help us pick between X and Y.

@loreanvictor
Copy link
Contributor Author

@theScottyJam previously made good points on this topic so I will defer to what was said there already.

This is indeed a good point (basically, increased coding safety). However, I cannot help but think this is something you could achieve with some linting rules instead of new syntax, similar to how the main power of const is when you add linting rules to enforce it (whenever possible).

@ljharb
Copy link
Member

ljharb commented Sep 10, 2021

It would be poor language design to abdicate responsibility for safety and push it onto the ecosystem, when it's achievable to avoid doing so.

@kaizhu256
Copy link

i think README.md should be updated with multi-use-temp-variable alternative mentioned at top. the doc seems a bit biased towards the java/haskell school-of-thought that javascript-variables should be "statically-typed" / single-use.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 10, 2021

It would be poor language design to abdicate responsibility for safety and push it onto the ecosystem, when it's achievable to avoid doing so.

I agree with the sentiment generally, though I feel in JS space people are super used to picking their own safety features (and thats why Prettier and TypeScript are super popular). I cannot argue one way or another about where the line should be drawn.

Hack-style pipelines are super close to assignment sequences using a token such as _. The reasons for lack of adoption of the latter in practice might (and most probably will) also affect the former. I am not saying safety of JS should be delegated to linters wholesale, I am saying if the main issue with assignment sequences was safety, perhaps we would have some linter rules for that (we already have linter rules to disallow unused arguments except when they are named _, for example). The fact that we don't, makes me be wary of the claim "the issue with assignment sequence is safety and hack-style pipelines fix that".

Regardless, if safety is an important benefit of hack-style pipeline pattern over using temporary variables, I suspect it at least should be mentioned in the README.


P.S. The primary actually used-in-practice pattern referenced for this proposal is jQuery or Mocha style method chaining. These patterns differ from hack-style in that they completely eliminate the need for a placeholder to begin with. I am all for standardizing and further facilitating commonly used design patterns in the language itself, but hack-style pipelining doesn't strike me as such a pattern.

@ljharb
Copy link
Member

ljharb commented Sep 10, 2021

@kaizhu256 so are most JS styleguides and best practices - variables shouldn't be reassigned.

@loreanvictor even if an identifier was viable (it is not), _ is a nonstarter due to the popularity of underscore and lodash - it's quite commonly used.

@loreanvictor
Copy link
Contributor Author

@kaizhu256 so are most JS styleguides and best practices - variables shouldn't be reassigned.

@loreanvictor even if an identifier was viable (it is not), _ is a nonstarter due to the popularity of underscore and lodash - it's quite commonly used.

Yeah I am using it as a mere example, not saying that this particular token has any benefits over ^ (though it visually does work better for me at least, and it is not an operator)

@kaizhu256
Copy link

@kaizhu256 so are most JS styleguides and best practices - variables shouldn't be reassigned.

at the macro/ux level of js-programming, temp-variables are reassigned all the time as placeholders for data waiting to be serialized/message-passed.

this proposal feels like an acknowledgement of above reality, but allowing ppl w/ dogmatic views like variables shouldn't be reassigned to break it using reassignable ^ instead.

@ljharb
Copy link
Member

ljharb commented Sep 10, 2021

@kaizhu256 i hear that that’s your experience, but in mine, in those exact use cases, variables are never reassigned.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 10, 2021

@kaizhu256 so are most JS styleguides and best practices - variables shouldn't be reassigned.

at the macro/ux level of js-programming, temp-variables are reassigned all the time as placeholders for data waiting to be serialized/message-passed.

this proposal feels like an acknowledgement of above reality, but allowing ppl w/ dogmatic views like variables shouldn't be reassigned to break it using reassignable ^ instead.

I don't think passing judgement on people participating in the discussion (such as calling their view dogmatic) helps advancing the discussion.

@kaizhu256
Copy link

but i do feel this proposal is political and advanced by ppl with certain dogmas on programming (static-typing and immutability of variables), but want to break that dogma in real-world programming, without having to admit to breaking it.

and increasing the language complexity for the rest of us.

@theScottyJam
Copy link

@kaizhu256 - careful - these people have very good reasons to believe the way they do about variable reassignment, just like you feel you have good reasons to believe variable reassignment is a good thing. An important part of language design is trying to figure out what patterns are good, what patterns are bad, and encouraging the good ones. They're currently under the belief (and so am I), that variable reassignment is generally a thing that should be avoided. Of course, these beliefs can be challenged, opposing scenarios can be brought up, and things can change.

But simply calling a particular viewpoint "dogmatic" is just hurtful and unhelpful.
It's hurtful because it's implying that everyone who has that viewpoint is uneducated and just following what's currently hip. In reality, these people have very good reasons to believe what they do, and throwing an "uneducated" label on them is just going to cause contention.
It's unhelpful because it doesn't actually contribute to an argument against the opposing viewpoint. You're claiming that the only reason they believe a certain way is because it's the hip thing to do, and you're not leaving room for an actual discussion about the pros and cons of variable reassignment.
It also just closes off the conversation. Any time someone here's "you're being dogmatic about X viewpoint", what they really here is "I don't see eye to eye with your viewpoint, and I think the only reason you don't believe the same way I do is because you haven't tried to understand me yet. You couldn't possibly give any good arguments to defend your viewpoint, so I feel safe in calling it 'dogmatic'". How could anyone defend a point of view if the opposing side has already decided it's 100% wrong and are unwilling to have a thoughtful discussion about it?

I wish, in the programming community as a whole, we would stop using that term. It never leads to good things.

@mAAdhaTTah
Copy link
Collaborator

"Prefer const" is an extremely common best-practice and hardly a minority dogma being imposed by this proposal. By comparison, in all the code & tutorials I've read or written, I have never once seen a single temp variable be reassigned over and over in a sequence of expressions. Reassignment of a variable once a message arrives is one thing, but it's not really what is being discussed here, and trying to conflate the two doesn't advance your argument.

@mAAdhaTTah
Copy link
Collaborator

Additionally, treating the placeholder as a "reassigned variable" is a mistake. It's not a general-purpose variable; it's explicitly the result of the LHS of the operator. Its value is narrowly defined, its scope is narrowly defined, and thus its purpose is narrowly defined. The placeholder is thus more akin to block scoping than temp variables, a feature I hope we can all agree is a significant improvement to the language.

@theScottyJam
Copy link

theScottyJam commented Sep 10, 2021

@loreanvictor - I think I'm following your argument, and I think what you're basically showing is that the README's argument for "variable naming" being the reason pipes are preferred over reassignment is simply not a great argument and perhaps needs to be updated. What you gave was pretty solid evidence that that argument by itself does not create enough merit to introduce a pipeline operator.

Security has been alternative reasoning that's been discussed around here. I personally view it as a readability thing more than anything - which I tried to explain thoroughly in the post that already got linked to over here. It's really easy to see a pipeline and know at a glance what's going on. It's much harder to look as a bunch of variable reassignments and know what's happening - variable reassignments have a lot more power to them, and can cause a lot of other things to happen that you might not expect. Pipelines, on the other hand, constrain the way you write your logic to follow a specific structure that can be understood at a glance and can't ever be deviated from.

I also believe that an important part of language design is to encourage good practices, and provide ways to make them easy to follow. Structuring code in a pipeline fashion is generally a good practice - code structured that way is generally more readable. For example, a function with a single pipeline is very easy to follow, and there's not a lot of stuff you have to keep in your head to folllow it - you just need to know how the previous step in the pipeline behaves in order to understand the next step. On the other hand, a function full of individual statements and many different variable names is much harder to follow. You have to keep in your mind a much bigger context - what the values of all of those different variables are at different points in time. The dependencies between one line and its predecessor lines are also much more complicated, due to the fact that any line can reference a variable created from any of the previous lines. There's also the fact that it's very, very easy to use side-effect inducing functions outside a pipeline. Inside a pipeline, programmers are naturally going to use pure-er functions, because you're forced to do something with whatever got returned from that function in the next stage of your pipeline. Using a temporary variable puts you into this kind of pattern, but doesn't force anything, and doesn't make it easy to see at a glance that this pattern is being followed.

@tabatkins
Copy link
Collaborator

but i do feel this proposal is political and advanced by ppl with certain dogmas on programming (static-typing and immutability of variables), but want to break that dogma in real-world programming, without having to admit to breaking it.

Speculating on motives is rarely productive or appropriate. In this case, it's wrong - I'm a primary champion, and don't feel either of those are particularly important. (But I recognize that it does affect a number of people, so it's not meaningless and should be considered.)

My counter-argument to this thread is simply: if people could do this today and use it to simplify their code in a meaningful way, why aren't they doing it? As far as I'm aware, this style of repeated re-assignment does not show up in any meaningful amount in the wild. Instead, they write difficult to read, heavily-nested code, or use uber-objects, or pipe(), etc.

There are a few possible conclusion to be drawn from that:

  • Maybe linearizing code isn't actually useful or important. I don't think this is true - the README goes into reasons for this, namely that people seem to really like patterns that do let them linearize code. It seems unlikely that there's something special about those specific patterns that doesn't generalize to the rest of the language.
  • Maybe people just really don't like repeated reassignment. This seems to be borne out by evidence; some reassignment is common in codebases (I commonly see function variables modified at the start of a function and re-assigned back to the same name, for instance; re-use of temp variables across separate contexts, where the two instances are semantically distinct things, like loop indexes, is also common), but heavy reassignment, particularly of this chained variety, seems to be avoided, either by linting/typing tools or just instinctually.

There's also a practical issue - re-assignment like this means that each pipeline step is seeing the exact same variable, so any closures over that variable will see the updated version from the end of the "pipeline", rather than the version that existed during their step. This is the same sort of footgun that used to plague callbacks created in for loops, and was solved by for-of doing some complex binding shenanigans to instead give the intuitive result.

<!DOCTYPE html>
<script>
var _ = 1;
_= (setTimeout(()=>console.log(_), 1), _),
_= 2*_,
_
// logs 2
</script>

The only way around this with named variables is to do what the example code in the README does, and use a unique name at each step. But that means you either have long, meaningful names, or short meaningless names that all look very similar, and this invokes the other issue with constantly naming steps - it makes it hard to determine at a glance what the scope of each variable is. Each one could be used in any of the succeeding steps; there's no guarantee that they're single-use in just the next line.


Summarizing my points:

  • people use and seem to enjoy multiple types of code-linearizing constructs, suggesting it's an important thing for them
  • but they avoid this precise construct (chained re-assignment of a temp variable) pretty consistently, suggesting that it has some vital faults that override its potential as a flexible code-linearizing construct in their minds
  • even if they did use it, it has some important limitations and footguns that aren't shared with the pipeline operator

@ljharb
Copy link
Member

ljharb commented Sep 10, 2021

I think the closure limitation is the clincher, really, and avoids the subjective arguments entirely.

@kosich
Copy link

kosich commented Sep 10, 2021

With all due respect:
Maybe, us not seeing such pattern used much in the wild, means that people don't really need this kind of chaining?
While we do see a lot of pipe( ) implementations, which means we have hard evidence that people need it.
Why do we invent stuff for hypothetical situations while we have hard evidence of devs using other approaches?

P.S: I wish we could see videos and/or notes from the September meeting, before things moved so fast in this repo. The lack of communication of the reasoning behind Hack over F# decision (before closing issues and updating readme & specs) doesn't let us fully and properly participate in the discussions.

@tabatkins
Copy link
Collaborator

Your argument in this thread was not over pipeline syntaxes, but over the need for a pipe operator at all. It applies equally well to any pipeline syntax.

@loreanvictor
Copy link
Contributor Author

Your argument in this thread was not over pipeline syntaxes, but over the need for a pipe operator at all. It applies equally well to any pipeline syntax.

I actually have to disagree. the argument I made was specifically around the hack-style pipeline. the F#-style pipeline doesn't have a token representing the preceding value, so this argument doesn't apply to it.

@loreanvictor
Copy link
Contributor Author

I feel my original question was "being too wordy doesn't seem like enough of an argument against temporary variables since we can make it equally wordy, are there any other reasons to prefer hack-style pipelines over temporary variables?" and we already have answers for that (basically code safety by avoiding variable reassignment since ^ is not a reassignable variable). I would recommend on formulating a real world-ish example highlighting that code safety difference, creating a PR to include this additional reasoning in the README as well, and then if there are arguments against those additional reasons, we can discuss them in separate issues afterwards (basically after they are well formulated and added as proper reasons for hack-style pipelines over temporary variables).

@tabatkins
Copy link
Collaborator

Hack-style and F#-style pipelines are (with some caveats) equivalent in power; anything you can do with one you can do with the other. Temp-var reassignment is an argument against the need for a pipe operator at all; it happens to resemble Hack-style pipelines more closely, but the argument for/against it is has zero details specific to Hack-style pipelines. The presence or absence of a placeholder token isn't significant; F#-pipeline absolutely does as well when you're using arrow functions (the arrow-func's argument). As far as I can tell, the argument for temp-var reassignment doesn't hinge on the alternative being point-free or not, either.

It's possible I'm missing something - can you elaborate on why you think temp-var reassignment is an argument against one pipe operator syntax in particular?

@theScottyJam
Copy link

Good point @tabatkins

I guess in other words, these are all equivalent:

const data = { x: 1, y: 2 }

const modifiedData = data
  |> Object.entries(^)
  |> ^.map(([k, v]) => [k, v + 1])
  |> Object.fromEntries(^)

const modifiedData = data
  |> _=> Object.entries(_)
  |> _=> _.map(([k, v]) => [k, v + 1])
  |> _=> Object.fromEntries(_)

let _ = data
_ = Object.entries(_)
_ = _.map(([k, v]) => [k, v + 1])
_ = Object.fromEntries(_)
const modifiedData = _

Given a properly structured temp-var pipeline, it's easy enough to do a direct conversion into either hack-style or F#-style. If hack-style pipes are seen as unecessary, because a temp var pipeline is a good enough replacement, then F#-style pipes should also be seen as unnecessary, as the differences between hack and F#-style are really quite minor (in F#, you use a function literal when you need topic-like behavior, in hack, you use a "(^)" when you're just wanting to use a single-param function - that's about it)

@ljharb
Copy link
Member

ljharb commented Sep 13, 2021

Same; they are quite different - if for no other reason then you can use the placeholder inside a closure, but if you use a temp var inside a closure you get silent bugs (the kind we're all very familiar with from decades of functions created inside for/for-in loops)

@loreanvictor
Copy link
Contributor Author

That's a reasonable summary, although I disagree with the claim that mutable temp vars are equivalent to the Hack topic token, which is intentionally constrained in scope & value and cannot be reassigned within a pipe step.

Same; they are quite different - if for no other reason then you can use the placeholder inside a closure, but if you use a temp var inside a closure you get silent bugs (the kind we're all very familiar with from decades of functions created inside for/for-in loops)

@ljharb @mAAdhaTTah I fully agree with these points. I tried to mention and elaborate on this difference in the Temp-var Reassignment is Unsafe section, with an example of such a silent bug that would be avoided with Hack-style pipelines. Did I miss anything? Or perhaps I can improve some wording somewhere for further clarity?

@mAAdhaTTah
Copy link
Collaborator

@loreanvictor

Which is the same with Hack-style pipeline version:

This is the statement I specifically object to in the summary, under "Mutable variables are harder to reason about". I disagree that the topic token is similar to a mutable variable, and if this summary is to be complete, it should include a explanation as to why that's the case.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 13, 2021

@loreanvictor

Which is the same with Hack-style pipeline version:

This is the statement I specifically object to in the summary, under "Mutable variables are harder to reason about". I disagree that the topic token is similar to a mutable variable, and if this summary is to be complete, it should include a explanation as to why that's the case.

@mAAdhaTTah What I meant is that to be able to reason about the ^ token in the third line, one would need to seek the ^) at the end of the second line, similar to how in the temp-var reassignment example, for reasoning about _ variable in the third line, one would need to seek the _) in the second line. Basically, the same way _ is a symbol that changes meaning throughout the first code, the ^ is a symbol that changes meaning throughout the second code.

@mAAdhaTTah
Copy link
Collaborator

What I meant is that to be able to reason about the ^ token in the third line

I don't think the third line is a good example in any style, tbh, and wouldn't be constructing arguments around it. In my original writeup, my suggestion was to write a function in all styles.

Basically, the same way _ is a symbol that changes meaning throughout the first code, the ^ is a symbol that changes meaning throughout the second code.

Yes, this is the claim I disagree with, because _ is scoped to the entire block and is mutable throughout, whereas ^ is immutable within the scope it appears (the RHS expression).

@loreanvictor
Copy link
Contributor Author

Yes, this is the claim I disagree with, because _ is scoped to the entire block and is mutable throughout, whereas ^ is immutable within the scope it appears (the RHS expression).

Do you agree with the fact that ^ has a different meaning (and references a different value) in this example depending on which line (or rather, pipeline step) you are reading?

  await chromeClient.rpc("Page.captureScreenshot", { format: "png" })
    |> (assertOrThrow(!^.exceptionDetails, ^.exceptionDetails), ^)
    |> await fs.writeFile("screenshot.png", Buffer.from(^.data, "base64"));

P.S. I do understand the scope difference and agree on that point, that is simply not the point I'm trying to make. I just find this code hard to read because I cannot map ^ to a singular thing and reason about the code, I need to take into consideration further context. That is the same readability issue arising from a mutable variable, you would need to also investigate the context to be able to reason about it.

@mAAdhaTTah
Copy link
Collaborator

mAAdhaTTah commented Sep 13, 2021

Do you agree with the fact that ^ has a different meaning (and references a different value) in this example depending on which line (or rather, pipeline step) you are reading?

Yes, but because that ^ is constrained in ways that a temp var are not, I have a lot less to scan to figure out the source & current value of ^ in each pipeline step, compared to a temp value.

@loreanvictor
Copy link
Contributor Author

Do you agree with the fact that ^ has a different meaning (and references a different value) in this example depending on which line (or rather, pipeline step) you are reading?

Yes, but because that ^ is constrained in ways that a temp var are not, I have a lot less to scan to figure out the source & current value of ^ in each pipeline step, compared to a temp value.

Good point. Can you also provide an example? All that comes to my mind are dirty in-place assignment expressions.

@theScottyJam
Copy link

theScottyJam commented Sep 13, 2021

Your argument is that "^" is difficult because you need to look to the previous line to know what it means.

Couldn't a very similar argument be said about F#-style?

What the function gets called with can't be determined unless you look at the previous line?

ok - maybe that's a weak rebuttal, but it's my thinking.


on a different note, I agree that when you format your code like this, it's very easy to tell that you're basically just doing a fake pipeline using existing tools.

let _
    _ = ...
    _ = ...
    _ = ...
const result = _

It gets much harder to follow that if code like this starts popping up:

let _
    _ = do {
      ...
      _ = 2 // Whoopsies
     ...
    }
    _ = _ + 1

And yes, that's a variation of your "dirty in-place assignment expression" example.

@mAAdhaTTah
Copy link
Collaborator

Good point. Can you also provide an example? All that comes to my mind are dirty in-place assignment expressions.

I think the example we've been discussing works fine. The original code snippet has data declared all the way at the top. I'd have to scan up to find out where that's declared & all the places its assigned before it gets to the step I'm looking at, as well as down to make sure any reassignment I'm doing doesn't affect downstream code. By comparison, in either pipe example, we only have to look on the exact line (and possibly the line directly before & after) to determine the impact a change on this line will have. I don't have a great example that isn't contrived tbh because I don't write code like that :).

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 13, 2021

Good point. Can you also provide an example? All that comes to my mind are dirty in-place assignment expressions.

I think the example we've been discussing works fine. The original code snippet has data declared all the way at the top. I'd have to scan up to find out where that's declared & all the places its assigned before it gets to the step I'm looking at, as well as down to make sure any reassignment I'm doing doesn't affect downstream code. By comparison, in either pipe example, we only have to look on the exact line (and possibly the line directly before & after) to determine the impact a change on this line will have. I don't have a great example that isn't contrived tbh because I don't write code like that :).

Yeah me neither, which is why it is a bit hard for me to also come up with similar examples. However, when contemplating potential increase in code complexity I would need to debug, I find it healthy to look at code outside of my own / preferred / linter-wouldn't-go-mad-at styles as well.

Anyways I updated the summary to reflect your point.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 13, 2021

Couldn't a very similar argument be said about F#-style?

@theScottyJam veering a bit off-topic, but,

The steps of the F#-style pipeline are fully independent from each other, so I don't need to read the previous step for reasoning about the next. However, with Hack-style pipelines, they have a dependency on each other in form of the ^, so you need to understand the previous pipeline step in order to be able to reason about the next (I guess that is the inevitable cost of the extra flexibility Hack-style provides).

However this is of course not a completely fair comparison, since in F#-style pipelines each step represents a higher-order construct (it is an expression resolving to another function, instead of a plain value), while in Hack-style pipelines each step resolves into a plain value. So if someone is not accustomed to reasoning about higher-order functions / expressions, perhaps they might find reading Hack-style pipelines easier still.

@ljharb
Copy link
Member

ljharb commented Sep 13, 2021

@loreanvictor that's not accurate. Every step in either pipeline depends on the output from the previous one. In F#, that dependence is implicit and invisible, but conventionally assumed in FP circles; in Hack that dependence is explicit with the ^.

@loreanvictor
Copy link
Contributor Author

loreanvictor commented Sep 13, 2021

@loreanvictor that's not accurate. Every step in either pipeline depends on the output from the previous one. In F#, that dependence is implicit and invisible, but conventionally assumed in FP circles; in Hack that dependence is explicit with the ^.

Yeah I tried to elaborate on this in the second paragraph as well. While this is true, I have (personally) got used to not trying to fully resolve the higher-order expressions of F# pipelines, which allows me to ignore the dependency, while in Hack-style, I feel kind of forced to deal with it heads on. Again, this should not be read as a strict pro for F# vs Hack, since it assumes being comfortable with reasoning about higher-order stuff, which statistically might not be a correct assumption (I highly suspect this is one of the main reasons people try to avoid RxJS).

@loreanvictor
Copy link
Contributor Author

Closing this issue since the all the advantages of Hack pipelines over mutable temporary variables are now listed in the README (see this and this section). I suspect further discussion on these points can be made in separate issues in a more focused manner.

@voliva
Copy link

voliva commented Sep 29, 2021

FWIW, the section on "pipelines resolve to expressions" hasn't considered that you can also have reassignments in expressions.

return (
  <ul>
    {
      values
       |> Object.keys(^)
       |> [...Array.from(new Set(^))]
       |> ^.map(envar => (
        <li onClick={() => doStuff(values)}>{envar}</li>
       ))
    }
  </ul>
)

can easily become

let $;
return (
  <ul>
    {
      ($= values,
       $= Object.keys($),
       $= [...Array.from(new Set($))],
       $= $.map(envar => (
        <li onClick={() => doStuff(values)}>{envar}</li>
       )))
    }
  </ul>
)

Again, not suggesting that this is a good practice or that Hack doesn't make this slightly better, but it kind of invalidates that point. Should I raise a PR amending that?

@theScottyJam
Copy link

Ah, that's true.

That'll let you do it in a lot of expressions. But not all.

let $;

const f = x => (
  $= x + 1
  $= $ + g($), // g() modified $, causing this to not work properly
  $= $ + 1
)

const g = x => (
  $= x + 1,
  $= $ * 2
)

Nested pipelines would also have an issue.

Well, I guess you could get around these issues if you used a different variable name for the topic variable. But yeah, this quickly becomes pretty gross.

@ljharb
Copy link
Member

ljharb commented Sep 29, 2021

@voliva because capturing $ in a closure would result in getting the wrong value, that would be a dangerous pattern to use or to recommend. Reassigning temporary variables are never an alternative to this proposal.

@js-choi
Copy link
Collaborator

js-choi commented Sep 29, 2021

FWIW, the section on "pipelines resolve to expressions" hasn't considered that you can also have reassignments in expressions.

Again, not suggesting that this is a good practice or that Hack doesn't make this slightly better, but it kind of invalidates that point. Should I raise a PR amending that?

Another pull request to improve the documentation’s examples would be welcome.

@js-choi js-choi added question Further information is requested documentation Improvements or additions to documentation labels Sep 29, 2021
@voliva
Copy link

voliva commented Sep 30, 2021

@ljharb

@voliva because capturing $ in a closure would result in getting the wrong value, that would be a dangerous pattern to use or to recommend. Reassigning temporary variables are never an alternative to this proposal.

Please, I explicitly said

Again, not suggesting that this is a good practice or that Hack doesn't make this slightly better

Of course it's not to be recommended. Of course it's dangerous. Of course it's buggy. Of course Hack has an advantage over reassigning.

I'm just pointing out that from this discussion, something was added on the README which isn't fully true.

@ljharb
Copy link
Member

ljharb commented Sep 30, 2021

It would be a very bad idea for the readme to imply that reassigning temp vars is a viable alternative - it’s far better to omit it. I’m probably missing something tho - if you think the readme can be improved, im sure the champions would appreciate a pull request.

@theScottyJam
Copy link

@ljharb, the concept of temporary variables is already in the README - @voliva is just wanting to improve the accuracy of what's already there.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests