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

include '.' on first line of multiline chained method invocations #176

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
@lcowell
Copy link

commented May 28, 2013

This changes where the . lives from the second line to the first line in the expression. Having no . at the end of the first line makes that line appear to be complete, but you must read ahead to the next line in order to see that the expression continues.

This closes issue #169

include '.' on first line of multiline chained method invocations
This changes where the '.' lives from the second line to the first line in the expression. Having no '.' at the end of the first line makes that line appear to be complete, but you must read ahead to the next line in order to see that the expression continues.

This closes issue #169
@bbatsov

This comment has been minimized.

Copy link
Collaborator

commented May 29, 2013

Since the . is at the beginning of the next line you're hardly required to do any reading ahead. Leading dots just stand out. I've never seen evidence that the alternative style you suggest is more popular than the one currently suggested.

@lcowell

This comment has been minimized.

Copy link
Author

commented May 30, 2013

You're right, you hardly have to read ahead, but you DO have to read ahead - and so does ruby. My argument against the next line dot is:
-it doesn't work at all in 1.8 (some people still use it)
-there is nothing to tell the reader to look at the next line (more ambiguous IMHO)
-it has a small performance penalty (https://gist.github.com/lcowell/3a63edecad51177857ec)

What do you think we should do ? I'd say our reasons are about equal. I like the trailing dot because it tells me to read the next line, you like the dot on the leading dot to make it stand out more.

Is there some process to handle situations like this or does the repo owner win ? I'd suggest that we include the pros and cons to each method in the guide, but then it has little value to include it at all and should probably just be eliminated altogether. Most importantly, let's not forget that our goal is to make ruby better and help people write better ruby code.

@rafamoreira

This comment has been minimized.

Copy link

commented May 30, 2013

👎 for trailing dots.

@jeromedalbert

This comment has been minimized.

Copy link
Contributor

commented May 30, 2013

I don't know about you guys but I like dots on the second line when combined with indentation :

# Example 1
one.two.three
       .four

# Example 2
my_array.select { |str| str.size > 5 }
        .map    { |str| str.downcase }

Identation + dot on the second line aligned with the previous one help me know visually (thus immediately) what is going on. There is no reading ahead in these cases.

@equivalent

This comment has been minimized.

Copy link
Contributor

commented May 30, 2013

+1 for this pull request

dot on beginning of line is inconstant with multi-line or or and #169 (comment)

@bbatsov

This comment has been minimized.

Copy link
Collaborator

commented May 30, 2013

@lcowell It's impossible to cater to everyone's preferences. If we simply list the two options available we're not actually recommending anything - and that's what style guides are for. Anyways, I'll keep this open for a while to hear what other people think as well. It's clear the opinions are greatly divided on that particular point.

@lcowell

This comment has been minimized.

Copy link
Author

commented May 31, 2013

@bbatsov agreed. No sense in including something in the guide if we're not making a clear recommendation. I was trying to make that same point above.

I like what @jdalbert was suggesting. Lining the dot up with the line above is an improvement does a better job of drawing the eye down than unaligned indentation. This addresses my primary concern with what's currently in the guide.

@bbatsov

This comment has been minimized.

Copy link
Collaborator

commented May 31, 2013

Yep, I like @jdalbert suggestion as well.

@lee-dohm

This comment has been minimized.

Copy link
Contributor

commented May 31, 2013

👍 on @jdalbert's suggestion.

@ghost

This comment has been minimized.

Copy link

commented May 31, 2013

I've been using trailing dots, with the second line indented.

As stated by others, the point is that when you read that line, you know that the statement continues on the next line. To me, that's more natural than reading a statement that looks complete in itself but actually continues in an oh-by-he-way fashion.

@jdalbert's solution fixes the problem with aesthetics but doesn't change the fact that the best place to indicate continuation of that statement is before you get to the next line.

# bad - need to consult first line to understand second line
one.two.three.
  four

# good - it's immediately clear what's going on the second line
one.two.three
  .four

I have two problems with this rationale. For one, you have to read both lines no matter what. You can't read four or .four and have any clue what's going on without reading the preceding line. The only way in which the second line might help with that is that it starts with a ., but if it's really just a matter of visually indicating that the previous line needs to be looked at, I think the fact that the line is indented does this fairly effectively.

Lastly, the second example is helpful only so far as you're reading or scanning the code from bottom to top -- which I suppose you could -- but I don't get why readability in that direction would be preferred over readability from top to bottom.

We want to make it clear that the second line goes with the first, but we don't want to make it clear that the first line goes with the second. This seems backwards. Trailing dot and second line indentation addresses the issue from either direction. This is a no-brainer if you ask me.

@fuadsaud fuadsaud referenced this pull request May 31, 2013

Closed

multiline chaining #169

@focused

This comment has been minimized.

Copy link

commented Jun 6, 2013

jdalbert, +1

@xjlu

This comment has been minimized.

Copy link

commented Jun 9, 2013

In real world programming, we have to read all lines anyway. The order of these method calls may or may not matter. With leading dots, editing is a bit easier, particularly when you need to add a new method call or change the order. (Similar to the javascript issue, you always have to remember to remove/add the trailing comma, so some developers prefer to put 'comma' in the beginning of each property.)

@focused

This comment has been minimized.

Copy link

commented Jun 9, 2013

When I start using an awesome gem for monads monadic, I realized that sometimes it is better to write dot at the end of line. Look (>= is an alias for bind):

Either(operation).
  >= { successful_method }.
  >= { failful_operation }
@philoserf

This comment has been minimized.

Copy link

commented Jun 18, 2013

i concur with the argument against.

#176 (comment)

bbatsov referenced this pull request in rubocop-hq/rubocop Jul 3, 2013

Bozhidar Batsov
@whitequark

This comment has been minimized.

Copy link

commented Jul 3, 2013

Leading dots are an antipattern:

  • They require the continuation to be present at precisely next line. (I.e. you cannot put more than one in-line comment in between.)
  • They break mental context by requiring one to always analyze one more line of code.
  • Most importantly, code written with leading dots cannot be copied and pasted to irb/pry.
@fuadsaud

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2013

@whitequark

  1. This is important.
  2. I don't understand why you wouldn't want to analyse the whole line. It's a single statement that would possibly be in one single line if not for it's length.
  3. Really neat, didn't think about that.

Anyway, I agree with @dingle, this is pretty similar to the leading/trailing comma in hashes. Each style has it's pros and cons; it's a matter of taste.

@whitequark

This comment has been minimized.

Copy link

commented Jul 4, 2013

@fuadsaud (2) Oh no, this is not what I meant. If the style guide forbids these leading dots, then I in my entire codebase, if a statement does not have a trailing dot, I know it ends right there.

Disagree on two counts.

  • Unlike comma in hashes, leading dots have objective differences--at least two of the three I recounted.
  • The very purpose of a style guide is to enforce a particular subjective style; and most often it makes sense to be conservative. You've already banned some constructs on similar grounds, e.g. and/or, or some %-literals, and rightly so--the resulting code is easier to read for everyone.
@prusswan

This comment has been minimized.

Copy link

commented Jul 5, 2013

I find leading dots more natural, because the first line (along with any number of lines that follow) will all be valid terminable statements. I feel this makes debugging and editing easier. This is also consistent with the idea of method-chaining (valid statements formed on top of valid statements).

@whitequark

This comment has been minimized.

Copy link

commented Jul 5, 2013

@prusswan You can always copy the fragment without the trailing dot to irb easily, but not with leading ones.

@prusswan

This comment has been minimized.

Copy link

commented Jul 5, 2013

@whitequark that is not a regular part of my workflow so no comments on that, but debugger and breakpoints work well enough for me.

@whitequark

This comment has been minimized.

Copy link

commented Jul 5, 2013

@prusswan I don't think leading/trailing dots have any effect whatsoever on debugging and breakpoints

@prusswan

This comment has been minimized.

Copy link

commented Jul 5, 2013

User.where
# .joins(...)
# ...
.group(...)
# ...

I can easily switch/comment out fragments and still retain a valid syntax. Autotest mechanism takes care of the execution.

@xjlu

This comment has been minimized.

Copy link

commented Jul 5, 2013

@whitequark

  1. I am not sure if it's still the case regarding the limitation of inline comments. Isn't that considered as a bug if it is as you described?
  2. The issue with irb/pry: isn't it a limitation of irb/pry? It doesn't make sense that we let a tool gem define a language's semantics.
@fuadsaud

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2013

@dingle the problem with irb is: when you paste the first line, irb receives the EOL and interprets the content as a full statement; that's the expected behaviour. When it sees the trailing dot, it interprets the content as a incomplete statement, therefore waiting for the rest to be pasted until it evaluates the code.

@whitequark

This comment has been minimized.

Copy link

commented Jul 5, 2013

@dingle

$ ruby -e $'1 # bar\n# baz\n  .display'
-e:3: syntax error, unexpected '.', expecting end-of-input
  .display
   ^
$ ruby -e $'1 # bar\n  .display'
1
$ ruby -v
ruby 2.0.0p0 (2013-02-24 revision 39474) [x86_64-linux]
@agis

This comment has been minimized.

Copy link

commented Aug 6, 2013

I'm 👎 on this one.

@rafaellima

This comment has been minimized.

Copy link

commented Sep 18, 2013

👎 it makes no sense to me.

@monfresh monfresh referenced this pull request Apr 16, 2014

Merged

Transpec #369

monfresh referenced this pull request in codeforamerica/ohana-web-search Jul 25, 2014

Tsagadai added a commit to Tsagadai/ruby-style-guide that referenced this pull request Oct 13, 2014

@kballenegger

This comment has been minimized.

Copy link

commented Dec 30, 2014

Has anybody considered this?

array.map(&:foo) \
  .reduce(&:bar)

Completely unambiguous, can look at either line knowing what's going on, can paste into REPL, etc.

@ddsferreira

This comment has been minimized.

Copy link

commented Mar 5, 2015

I believe this style is misguiding.
I also believe we should avoid long method chains.
And provided we have self contained method chains I would prefer a single line approach:
foo(arg1).bar(arg2).baz(arg3, arg4)
I would also add a maximum configuration on this upon which an error/alert should be raised by rubocop.
In my opinion the default maximum should be 3.

@elliottmason

This comment has been minimized.

Copy link

commented Mar 18, 2015

@kballenegger 's approach is arguably a nice compromise between the two styles. The trailing slash indicates that the statement continues, while still allowing one to use preceding dots, and it still works in IRB. Additionally, it doesn't require any configuration changes to Rubocop. I'm happy with it.

I agree that we should be avoiding long method chains in most scenarios. However, sometimes it's not the number of methods being chained that causes one to have to continue a statement on the next line, but merely the number of characters in a statement.

@roryokane

This comment has been minimized.

Copy link

commented Apr 7, 2015

I have compiled all the arguments I could find in this thread into a comprehensive list of pros and cons. All four styles described in this thread are compared:

  • . at line end
  • . at line beginning
  • . at line beginning, indented to the previous .
  • \ at line end, . at next line’s beginning

The pros and cons are kind of long, so I’ll just link to them on Stack Overflow.

@justin808

This comment has been minimized.

Copy link

commented Apr 9, 2015

@bbatsov I see the change: rubocop-hq/rubocop@e41175f

Is the default to be beginning of line or end of line? Seems to be.

Thanks.

@bbatsov

This comment has been minimized.

Copy link
Collaborator

commented Apr 9, 2015

s the default to be beginning of line or end of line? Seems to be.

Yes. See default.yml for alternatives.

@bb010g

This comment has been minimized.

Copy link

commented May 13, 2015

@focused commented on Jun 9, 2013, 2:21 AM PDT:

When I start using an awesome gem for monads monadic, I realized that sometimes it is better to write dot at the end of line. Look (>= is an alias for bind):

Either(operation).
  >= { successful_method }.
  >= { failful_operation }</pre></div>

I've always seen it in Haskell as (looks better with bind instead of >= here IMHO)

monadic_operation.bind ->(foo)
  {bar(foo, 1)}.bind ->(biz)
  {baz(biz, 2)}

which would be equivalent to the following in Haskell do-notation

do
  foo <- monadicOperation
  biz <- bar foo 1
  baz biz 2

@GreenCheese GreenCheese referenced this pull request Jun 9, 2015

Closed

Fixed typos #450

marocchino pushed a commit to marocchino/ruby-style-guide that referenced this pull request Aug 11, 2015

shyouhei pushed a commit to shyouhei/ruby-style-guide that referenced this pull request Nov 11, 2015

@kashif-umair

This comment has been minimized.

Copy link

commented Aug 23, 2016

I have a question about blocks with these chaining methods. I'm using Rails so my example is specific to Rails but rails is Ruby after all. So here it is:

user.posts.includes(:comments).where(published: true).where.not(post_category: 'Private').each do |post|
  # do something with post here
end

What is the best way to write the above code? I do something like this:

user.posts.includes(:comments).
where(published: true).
where.not(post_category: 'Private').each do |post|
  # do something with post here
end

I prefer to use trailing dots but if look at my code, I have not indented line number 2 and 3 because if I indent them then I have to add one more indentation level to each block.

@equivalent

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2016

I would say:

user
  .posts
  .includes(:comments)
  .where(published: true)
  .where.not(post_category: 'Private')
  .each do |post|
     # do something with post here
  end

or

user.
  posts.
  includes(:comments).
  where(published: true).
  where.not(post_category: 'Private').
  each do |post|
     # do something with post here
  end

I would prefer the first version

reason for having it in separate lines is that it's easier to alter (e.g. you add .order(:created_at) the commit message will be just one line in git log instead of long line where is not clear what have changed.

.where.not are together on a same line as they are logically bound together

@FranklinYu

This comment has been minimized.

Copy link

commented Nov 19, 2016

I have not indented line number 2 and 3 because if I indent them then I have to add one more indentation level to each block.

@kashif-umair I think you may as well just indent line 2 and 3, making each block indented one more level, because that makes sense: one level for continuation, another for nesting. In ruby code, I believe indenting 5 levels are very common.

@cyoce

This comment has been minimized.

Copy link

commented Jan 24, 2017

@bbatsov can you please put option C in the style guide? It is a nice compromise.

one \
.two
@desmond-silveira

This comment has been minimized.

Copy link

commented Mar 15, 2017

In any language where line terminators are mandatory, like Java, the benefits of the leading . outweigh the benefits of the trailing .. It's just easier for a human to see the . at the beginning of a line than at the end.

However, in any language where line terminators are optional, like Ruby, the benefits of the trailing . outweigh the benefits of the leading .. The ability to paste code into a REPL is just too important.

@mhenrixon

This comment has been minimized.

Copy link

commented Mar 20, 2017

Very interesting discussion! Personally I always had a very strong preference for the first option with the leading . but for me it is just how my brain is wired. It is also how I prefer to work with other languages like SQL where I prefer the , to be in the beginning of the line to more easily be able to add and remove lines without having to also worry about adding a trailing comma somewhere.

Never thought of the option C that is definitely my new preference!

Many thanks to everyone for the lengthy and thorough discussion.

@Startouf

This comment has been minimized.

Copy link

commented Apr 6, 2017

Ah wouldn't it be just so great if Ruby could read syntax like this one

some_data.
.map { |d| d**2 }.
.sort_by { |d| d.odd? }.
.reject { |d| d.zero? }.
.reduce(:+)

it would be a nice compromise (that would looks better than the one mixing \and .)

As far as I know the .. operator is only used for date ranges but never broken by a \n so shouldn't that be theoretically feasible in future versions of Ruby ?

Also, and this most likely a personal feeling, but I don't like the indentation after the first line breaks, whatever the choice for the placement of the dot. But in case of leading dots it feels obvious enough that the chain continues from the previous lines that I don't see the pros of indentation.

On the contrary I would argue extra indentation makes it harder to close blocks. Take the following example of chaining with multiline blocks (putting aside the code smell), it makes it harder to realize if you're missing an end or not. I've quite several times struggled to find the missing end of code blocks, and having code that offsets itself for no reason makes it more error-prone. Maybe my linters are just bad, but often I would just get a red rubocop error at the end of the file and I'd have to find the missing/extra end myself

def get_questions
  Question.
    where(:score.gte => 42).
    pluck(:answer).
    select do |answer|
      answer.is_the_answer_to_life_and_everything?
    end.uniq
  # If not paying attention I'd want to put an `end` here
end

@ivar ivar referenced this pull request Sep 11, 2018

Merged

updates rubocop config #3462

marcotc added a commit to wealthsimple/ws-style that referenced this pull request Feb 12, 2019

Use trailing dots
There's a long discussion (rubocop-hq/ruby-style-guide#176 (comment)) in the Rubocop community around this and the consensus is to "be consistent", no matter if you choose trailing or leading dots.

At Wealthsimple there's a considerable disadvantage to using leading dots: code with leading dots cannot be pasted into irb/pry. This affects local testing and scripting.

We'd had bugs in the past related to people pasting large snippets from a perfectly functioning Ruby file into pry, but that script fails very subtly, amidst hundreds of successfully pasted lines, one fails but the whole block is still successful.
Example:
```ruby
# a lot of code here
list = bar.map{|x|x+1}
         .select{|x|x>1}
# much more code here
```
this code still "works", but won't give you the correct result.

@marcotc marcotc referenced this pull request Feb 12, 2019

Merged

Use trailing dots #12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.