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

style guide: revise sections on infix and postfix notation #716

Merged
merged 2 commits into from Mar 2, 2017
Merged

style guide: revise sections on infix and postfix notation #716

merged 2 commits into from Mar 2, 2017

Conversation

SethTisue
Copy link
Member

the postfix text is mostly the same, but now comes last and uses the
word "postfix" instead of "suffix".

the infix text is totally rewritten and now more strongly discourages
use of infix for alphabetic method names. back when the style guide
was first written, I think there was more experimentation with, and
more acceptance of, such use of infix notation. these days, not as
much.

I omitted the material about mixing dot and dotless notation -- the "a
b.(c) d" example. I found that passage highly confusing and I've seen
others be confused by it as well. I don't think it's important to
include.

review by @WadeWaldron and community

the postfix text is mostly the same, but now uses the word "postfix"
instead of "suffix"

the infix text is totally rewritten and now more strongly discourages
use of infix for alphabetic method names.  back when the style guide
was first written, I think there was more experimentation with, and
more acceptance of, such use of infix notation.  these days, not so
much.

I omitted the material about mixing dot and dotless notation -- the "a
b.(c) d" example.  I found that passage highly confusing and I've seen
others be confused by it as well.
@SethTisue
Copy link
Member Author

the diffs will be easier to review if you look at the two commits separately.

Copy link

@WadeWaldron WadeWaldron left a comment

Choose a reason for hiding this comment

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

This seems much more clear. Thank you.

These changes seem to fall more in line with modern Scala style. It matches better with other style guides such as Databrix and Paypal:

https://github.com/databricks/scala-style-guide#infix
https://github.com/paypal/scala-style-guide#calling-functions-1

@Ichoran
Copy link

Ichoran commented Feb 27, 2017

Looks good!

@dwijnand
Copy link
Member

RIP. :-(

@vpetro
Copy link

vpetro commented Feb 28, 2017

👍

@SethTisue SethTisue merged commit 393e190 into scala:master Mar 2, 2017
@SethTisue
Copy link
Member Author

thanks, reviewers

@SethTisue SethTisue deleted the infix-and-postfix branch March 2, 2017 19:38
@SethTisue
Copy link
Member Author

cc @djspiewak whose prose I think I recognize here :-)

@som-snytt
Copy link
Contributor

I thought the better-known names for /: and :\ were querulous and chagrin, respectively.

@djspiewak
Copy link
Member

@SethTisue This is indeed my prose. :-) I agree that infix alphabetical function usage is now far less common than it used to be, but I still strongly believe it is the most consistent style. Consider the following example:

xs map { a => a + a } filter { _ < 12 }

xs.map { a => a + a }.filter { _ < 12 }

xs.map(a => a + a).filter(_ < 12)

Let's just agree right now that the second form is absurd and shouldn't be encouraged. If nothing else, "dotting" on the end of a lambda sometimes doesn't parse the way you would expect (e.g. the pendingUntilFixed method in specs2).

From a personal aesthetic standpoint, I strongly prefer the third option. It's the most concise, feels the most "functional" (in that it treats lambdas as normal values), avoids the subjectively-ugly curly brace, scales well (you can just keep chaining functions without parentheses, regardless of arity), is amenable to explicit type parameters and currying, etc. There's a ton of benefits! Where it falls over though is if you swap the lambda passed to map:

xs map {
  case a => a + a
} filter { _ < 12 }

xs.map {
  case a => a + a
}.filter { _ < 12 }   // not even sure if this parses correctly

xs.map({
  case a => a + a
}).filter(_ < 12)

You see the problem. I've never heard anyone give me a plausible solution to this issue.

There's a secondary problem here as well, which is what happens if you aren't switching to a partial function, but you've discovered that your lambda is now non-trivial and needs an intermediate value:

xs map { a => 
  val back = a + a
  back
} filter { _ < 12 }

xs.map({ a => 
  val back = a + a
  back
}).filter(_ < 12)

A more common pitfall here is to try to inject a println(a); prior to the a + a, which completely breaks if you use parentheses since the lambda will end at the ;, rather than at the ). Neither of these are issues for curly braces.

So every time I come back to this problem, I really want to use dot notation for alphabetical functions and parentheses for lambdas. I really really want to. But curly braces are unavoidable, both for partial functions and for multi-line lambdas, and they just don't play nicely with a paren- and dot-focused syntactic idiom.

For this reason, I still strongly advocate for an infix style on alphabetical functions. At the very least alphabetical functions which may be used in conjunction with other alphabetical functions which take lambdas.

@SethTisue
Copy link
Member Author

SethTisue commented Mar 2, 2017

I thought the better-known names for /: and :\ were querulous and chagrin, respectively.

those were the eighth and ninth dwarves in one of Walt's early drafts, I think

@djspiewak your "not even sure if this parses correctly" does parse correctly and works fine (and reads fine too IMO), so I don't see the problem. dots and curlies are compatible, so wanting curlies never means losing dots, afaict. and I've never seen a case where it was necessary or appropriate to resort to ({...})

@djspiewak
Copy link
Member

@djspiewak your "not even sure if this parses correctly" does parse correctly and works fine (and reads fine too IMO), so I don't see the problem. dots and curlies are compatible, so wanting curlies never means losing dots, afaict. and I've never seen a case where it was necessary or appropriate to resort to ({...})

Example from specs2:

"do the things" in {
  "stuff" must haveLength(5)
}.pendingUntilFixed

Play around with different associations of parentheses, dots, lack of dots, etc. The target of the pendingUntilFixed dispatch is highly unintuitive.

@SethTisue
Copy link
Member Author

SethTisue commented Mar 2, 2017

ah. well, ugh on these English-y DSLs in first place, to my personal taste

in a codebase where the use of the infix style for this DSL was considered standard, I would write your example as

("do the things" in {
  "stuff" must haveLength(5)
}).pendingUntilFixed

and now I understand the motivation behind the original text better. regardless, I think it's a niche enough issue that I'm comfortable omitting it from the guide.

@som-snytt
Copy link
Contributor

I don't know if my understanding as a learner clarifies the earlier comment about braces, but I agree that receiver op { body }.f is hard to see.

To say more, f can take (args) or { stms ; res }, and that made sense to me.

@odersky
Copy link
Contributor

odersky commented Mar 2, 2017

I also have reverted almost completely to explicit "." style. it's just too much to choose between the two all the time, and I found that "." style is most often more legible, and flows better when writing it (i.e. fewer times to go back and insert parentheses). When that's too revolting (as it is for an inherently commutative binary operator) I have started to use backticks to distinguish the operator from other text. Example:

x `min` y

instead of

x min y

or (ugh!)

x.min(y)

To be honest, I am not sure I'll keep this up, or whether it will be just a fad :-)

The one exception is that I always write method names infix when followed by {, because then they look like control structures. @djspiewak: It seems that takes care of all of your counter-examples?

@som-snytt
Copy link
Contributor

In recent contributions, I have restored dots where I had to think twice about the scansion.

I started to think twice about it after the "Scala style" keynote from a couple of years ago, so it is actually a long process to change a way of seeing code. So leadership in setting the pace is appreciated.

I would expect backticked max is a tic, however.

Maybe backtick is easier on a Swiss keyboard. And in emacs. For vi, it is dangerously close to ESC, especially on a laptop with chiclet keys.

@fommil
Copy link

fommil commented Mar 2, 2017

I've encountered unexpected behaviour when using {...} curly notation when combined with _ underscore. Is possible to introduce multi lines where the first line is evaluated only once, not on every evocation of the lambda (as one would expect). Yes, it was nasty horrible side-effecting actor code, but it was enough to scar me for life and I discourage any use of both together now. Underscore and parens, or use a curly with an explicit parameter!

@Ichoran
Copy link

Ichoran commented Mar 2, 2017

@djspiewak - All of my code is the second form, save for whitespace, and it's utterly readable and not the least bit absurd.

  myData.
    map{ x => someOperationOn(x) + someOtherOperationOn(x) }.
    filter{ y => foo(y) && !bar(y) }

The nice thing about this form is that it's infinitely indentable--just drop the parens down a line and keep going:

  myData.
    map {
      case x if small(x) => someOperationOn(x)
      case x => someOtherOperation(x)
    }.
    filter(_ < 12)

Whether or not this is recommended style (I don't care!), I don't instantly agree that it's absurd, or I wouldn't be doing it everywhere :P

@non
Copy link
Contributor

non commented Mar 2, 2017

Yeah, a variant of the second style that we use frequently is something like this:

source
  .map { x =>
    ...
  }
  .filter { y =>
    ...
  }
  .grouped

@Ichoran
Copy link

Ichoran commented Mar 2, 2017

@non - I do a lot of development by-paste-in-REPL, and that form doesn't work there.

@dwijnand
Copy link
Member

dwijnand commented Mar 2, 2017

@Ichoran You could wrap it in parens as you're pasting it in..

@fommil Good point. As an illustration:

scala> List(1, 2) map { println("hi"); _ + 1 }
hi
res6: List[Int] = List(2, 3)

vs

scala> List(1, 2) map { x => println("hi"); x + 1 }
hi
hi
res7: List[Int] = List(2, 3)

@dwijnand
Copy link
Member

dwijnand commented Mar 2, 2017

My gripe with dot-notation is you can't use it for match or yield. And that it's more noisy that whitespace.

@djspiewak
Copy link
Member

djspiewak commented Mar 2, 2017

@odersky

The one exception is that I always write method names infix when followed by {, because then they look like control structures. @djspiewak: It seems that takes care of all of your counter-examples?

Maybe I should just get used to what you're suggesting. Technically, my current style is already inconsistent in the case of multiple parameter blocks or explicit type parameter instantiations (since those don't work with infix or postfix style). What you're suggesting is inconsistent w.r.t. the dispatch "operator" (space vs dot), but perhaps it's less inconsistent overall. And it does enable the more pleasing parenthetical style.

@fommil

I've encountered unexpected behaviour when using {...} curly notation when combined with _ underscore. Is possible to introduce multi lines where the first line is evaluated only once, not on every evocation of the lambda (as one would expect). Yes, it was nasty horrible side-effecting actor code, but it was enough to scar me for life and I discourage any use of both together now. Underscore and parens, or use a curly with an explicit parameter!

I've had the opposite issue. To wit:

xs.map(a => println(a); a + a)

I think I pointed this one out in the original comment. Or, if you favor underscores:

xs.foldLeft(0)(println(_); _ + 1)

Also this seems like an appropriate time to bring up my favorite Scala syntactic puzzler of all time:

xs.foldLeft(0) _ + _

…which actually compiles!

@Ichoran

All of my code is the second form, save for whitespace, and it's utterly readable and not the least bit absurd.

Maybe this is a taste thing, but I legitimately have a hard(er) time reading your snippet. Especially with the unspaced open curlies and the trailing dots.

@non

Yeah, a variant of the second style that we use frequently is something like this

I've used that style. Sometimes you have to, because of the way newline inference special-cases leading dots.

@dwijnand

My gripe with dot-notation is you can't use it for match or yield. And that it's more noisy that whitespace.

I don't think of yield or match as being functions, so that works out pretty well for me. :-) So I would keep them infix for exactly the same reason that I would continue to put a space between the if and the ( character. Also, even if match were a function (scala trivia note: it used to be!), infix form would fall out of @odersky's proposed stylistic distinction.

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

Successfully merging this pull request may close these issues.

None yet

10 participants