Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
lcowell wants to merge1 commit intorubocop:masterfromlcowell:master

Conversation

lcowell
Copy link

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

TylerRick reacted with thumbs up emojiakimd reacted with thumbs down emojirafaelsales, jonahx, and Elshazly888 reacted with heart emoji
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 issuerubocop#169
@bbatsov
Copy link
Collaborator

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.

RudyOnRails, ianbayne, gregmc-omada, khiav223577, P3t3rU5, akimd, and jontsai reacted with heart emoji

@lcowell
Copy link
Author

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
Copy link

👎 for trailing dots.

@jeromedalbert
Copy link
Contributor

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

# Example 1one.two.three.four# Example 2my_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.

Foozie3Moons, yeniv, khiav223577, valmirosjunior, P3t3rU5, ahmedfawzy98, akimd, nafaabout, TylerRick, hokageCV, and 3 more reacted with thumbs up emoji

@equivalent
Copy link
Contributor

+1 for this pull request

dot on beginning of line is inconstant with multi-lineor orand#169 (comment)

@bbatsov
Copy link
Collaborator

@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.

ivar, easbarba, jedeleh, and jonahx reacted with thumbs up emojiaadeshere1 and ahmedfawzy98 reacted with laugh emoji

@lcowell
Copy link
Author

@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
Copy link
Collaborator

Yep, I like @jdalbert suggestion as well.

@lee-dohm
Copy link
Contributor

👍 on @jdalbert's suggestion.

@ghost
Copy link

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 isbefore you get to the next line.

# bad - need to consult first line to understand second lineone.two.three.four# good - it's immediately clear what's going on the second lineone.two.three.four

I have two problems with this rationale. For one, you have to read both lines no matter what. You can't readfour 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.

amiel, stevo, 3limin4t0r, viloner, andrii, SnailCoil, evanthegrayt, michael-gillett, sureshtadisetty, nicolas-fricke, and 5 more reacted with thumbs up emojiFoozie3Moons and akimd reacted with thumbs down emoji

@fuadsaudfuadsaud mentioned this pull requestMay 31, 2013
@focused
Copy link

jdalbert, +1

@xjlu
Copy link

xjlu commentedJun 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
Copy link

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

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

@philoserf
Copy link

i concur with the argument against.

#176 (comment)

bbatsov referenced this pull request in rubocop/rubocopJul 3, 2013
@whitequark
Copy link

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.
booch, pwim, ajsharma, b264, gnapse, troex, desmond-silveira, Startouf, edbond, amiel, and 28 more reacted with thumbs up emojijonahx reacted with thumbs down emoji

@fuadsaud
Copy link
Contributor

@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
Copy link

@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.
  • Thevery 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
Copy link

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).

ilkecan and TylerRick reacted with thumbs up emoji

@whitequark
Copy link

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

@prusswan
Copy link

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

@whitequark
Copy link

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

@prusswan
Copy link

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

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

ilkecan, austin-leligdon, and camp-sai reacted with thumbs up emoji

@xjlu
Copy link

xjlu commentedJul 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.
P3t3rU5 and killerkiara reacted with thumbs up emoji

@fuadsaud
Copy link
Contributor

@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
Copy link

@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 -vruby 2.0.0p0 (2013-02-24 revision 39474) [x86_64-linux]

@agis
Copy link

agis commentedAug 6, 2013

I'm 👎 on this one.

@rafaellima
Copy link

👎 it makes no sense to me.

@ddsferreira
Copy link

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.

GeometerGames, akimd, jonahx, michaelfranzl, and pioz reacted with thumbs down emoji

@elliottmason
Copy link

@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
Copy link

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 justlink to them on Stack Overflow.

10XL, N0xFF, esquinas, and jonahx reacted with thumbs up emoji

@justin808
Copy link

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

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

Thanks.

@bbatsov
Copy link
Collaborator

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

Yes. Seedefault.yml for alternatives.

@bb010g
Copy link

@focused commented onJun 9, 2013, 2:21 AM PDT:

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

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

I've always seen it in Haskell as (looks better withbind 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 foo1  baz biz2

@kashif-umair
Copy link

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').eachdo |post|# do something with post hereend

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').eachdo |post|# do something with post hereend

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 toeach block.

@equivalent
Copy link
Contributor

equivalent commentedAug 24, 2016
edited
Loading

I would say:

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

or

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

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
Copy link

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

@kashif-umair I think you may as well just indent line 2 and 3, makingeach 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
Copy link

cyoce commentedJan 24, 2017
edited
Loading

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

one \.two
kballenegger, Startouf, booch, and michaelfranzl reacted with thumbs up emojiknodi, andrii, jcypret, justincampbell, juankiz, Foozie3Moons, evanthegrayt, aniamoto, timbreitkreutz, P3t3rU5, and marcandre reacted with thumbs down emojiNewAlexandria, cbrwizard, and mhenrixon reacted with confused emoji

@desmond-silveira
Copy link

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.

Domon, bb010g, Startouf, amiel, ChaelCodes, ptoth88, joshua-honig, Foozie3Moons, evanthegrayt, agbodike, and 5 more reacted with thumbs up emoji

@mhenrixon
Copy link

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 optionC that is definitely my new preference!

Many thanks to everyone for the lengthy and thorough discussion.

@Startouf
Copy link

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 anend or not. I've quite several times struggled to find the missingend 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/extraend 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` hereend
FranklinYu, rubendinho, roryokane, rhovhannisyan-vineti, and P3t3rU5 reacted with thumbs up emojinafaabout reacted with thumbs down emoji

marcotc added a commit to wealthsimple/ws-style that referenced this pull requestFeb 12, 2019
There's a long discussion (rubocop/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 herelist = 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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@CheloupCheloupCheloup approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

31 participants
@lcowell@bbatsov@rafamoreira@jeromedalbert@equivalent@lee-dohm@focused@xjlu@philoserf@whitequark@fuadsaud@prusswan@agis@rafaellima@nedzadarek@bf4@dgutov@steveklabnik@kballenegger@ddsferreira@elliottmason@roryokane@justin808@bb010g@kashif-umair@FranklinYu@cyoce@desmond-silveira@mhenrixon@Startouf@Cheloup

[8]ページ先頭

©2009-2025 Movatter.jp