Github Ruby Styleguide

https://github.com/styleguide/ruby

by fox91 • 14 years ago

88 69 14 years ago

github.com

NateDad2• 14 years ago

I'm not a Ruby programmer, but a couple of these things just look completely wrong:

# good

def some_method(some_arr)

  some_arr.size

end

Omitting the return statement is a good thing? How do you know if this method even returns anything? .size could be a method that doesn't return anything, right, since parens are optional?

#good

do_something if something_else

This, to me, is horrible.

If you're just scanning down the file, you can easily miss the if statement, and assume the do_something always goes off.

Also, it's incredibly hard to process when I'm mentally executing the code in my head. Generally if there's a method call on the left, you just do it... but now we hit this if statement, ok, so mentally undo the method call we just processed, now check the if statement... now skip back to the do_something and reapply the method call... and now skip the if statement and continue on in the code.

...or you could just do if something_else then do_something (if you really must have single line if statements, which I am also against, but not so much as the if statement after the method it's controlling access to)

lucian1900• 14 years ago

Returning the last expression is quite handy for a lot of functional patterns. One gets used to it very quickly, especially one has used lisp at all.

Parens being optional is indeed a flaw in Ruby's design.

Post-if can be handy, sometimes.

xentronium4• 14 years ago

> Parens being optional is indeed a flaw in Ruby's design.

This flaw allows us to have nice DSLs unencumbered with parens.

matt_yoho• 14 years ago

> Omitting the return statement is a good thing? How do you know if this method even returns anything?

In Ruby, all methods return the value of the last expression executed in the method.

> .size could be a method that doesn't return anything, right, since parens are optional?

This is in theory true but never happens in practice.

Edit: Ruby has no "non-method" properties on objects. Everything is a method call, regardless of use of parens.

> If you're just scanning down the file, you can easily miss the if statement, and assume the do_something always goes off.

An experienced Rubyist would catch it, if not at first glance, with the second. This is used relatively sparingly, and in scenarios conceptually similar to an early bailout return statement. It makes sense where it makes sense, if you will.

Neither of these is a problem in practice. It may be an issue of familiarity and comfort with the language, but in my experience it doesn't take long to be comfortable with either of those conventions.

xentronium4• 14 years ago

> Omitting the return statement is a good thing? How do you know if this method even returns anything?

All methods return something.

> If you're just scanning down the file, you can easily miss the if statement, and assume the do_something always goes off.

This is actually one of the arguable points in that styleguide. Some people share your disapproval for inline if modifiers. Anyway it's not that bad after getting used to as long as lines aren't too long and predicates aren't too complex.

NateDad2• 14 years ago

> All methods return something.

Ahh, that helps, thanks.

masklinn2• 14 years ago

> .size could be a method that doesn't return anything, right, since parens are optional?

It can only be a method. Instance attributes can not be public in ruby (and are delineated by the `@` prefix sigil), the closest equivalent (attr_reader/attr_writer/attr_accessor) is a dynamically generated method (or pair of methods)

lloeki2• 14 years ago

> I'm not a Ruby programmer

and (without any form of negative judgement), this is why

> a couple of these things just look completely wrong

to you, because they're idiomatic of Ruby.

First, lack of return.

A random example in Rails's ActiveSupport[0]:

    def to_s
      "(GMT#{formatted_offset}) #{name}"
    end

It just reads like ":to_s is defined as being a String constructed this way". Note that formatted_offset and name are resolved as methods in the context (i.e self), and self. is simply omitted, which is also idiomatic.

Not only functions always return something (their last statement's return value), every statement actually returns something, so

    foo = if a == b then 1 else 2

works and is used, but also with case/when, and whatever you can think of. With proper indentation this is perfectly sane and readable, and avoid redundant, 'side effect' (from the point of view of code flow) assignments, easing comprehension and refactoring, while making mistakes less likely (foo will always and obviously be assigned something).

What's more, a Ruby construct known as blocks are used with methods (such as Enumerable#map or Enumerable#select) needing return values, and omitting return is a readability and typing boon.

That's not to say return has no use, but then when you use it, you mean it, so that it is used in match/exit pattern methods, where you're doing some checks, and dispatch-return according to various situations, i.e you return early.

It is also used in Procs, which differ from lambdas in that a return will return from the caller, not the Proc.

Second, right-side if.

This aids readability when you're concerned about doing a number of sequential tasks, and some of them are conditioned. All possible tasks are aligned, and all conditions are on the right side. An absence of condition means "always". Sounds almost like column-based source code (fortran, RPG on AS400...).

You mention scanning down the file, but idiomatic ruby would not make you scan down the file: you would be faced with a function, whose size would make its structure/pattern apparent, and with such a pattern the logic would be readily seen, dangling on the right side.

Such conditions are often used with the first part of this discussion, so that you see immediately what is returned right there on the left, possibly conditioned.

[0] https://github.com/rails/rails/blob/5fe88b11f11bb3b30bc23c57...

yalue2• 14 years ago

I agree with almost everything except for using named groups over the $1-$9 variables in regular expression matching. This may be the case if there are many capture groups in the expression, but I would argue that

    /(?<meaningful_var>regexp)/ =~ string

is a fair deal more difficult to read than simply

    /(regexp)/ =~ string

Almost everybody familiar with PCRE will be familiar with the simpler form, and it's usually the case that the shorter the regular expression, the easier it is to understand and read.

fox912• 14 years ago

I think it's good to name these variables, obviously you need to use short names, otherwise you'll get very long regexps. but in any case it will increase readability imho.

viseztrance• 14 years ago

The following are equivalent:

  if /(?<meaningful_var>regexp)/ =~ string
    puts meaningful_var
  end

  if /(regexp)/ =~ string
    puts $1
  end

This may not look like much, but bare in mind you may be using the current selection in several places, at which point you'll probably use the following for readability either way:

  meaningful_var = $1

In any case I think that sometimes the latter is preferable, that's why I'm not into these kind of black and white conventions. OMG, this line is 85 characters, you suck.

yalue2• 14 years ago

It is true that one will probably immediately reassign the $- variables. And that's definitely true that there are cases when conventions impede good style. I would probably say, though, that in most cases the latter of your examples is preferable.

    if string =~ /First: (.*?) Last: (.*?)\s/
      first_name = $1
      last_name = $2
      # etc...
    end

looks far better to me than

    if string =~ /First: (?<first_name>.*?) Last: (?<last_name>.*?)\s/
      # etc...
    end

Karunamon2• 14 years ago

I'm a little curious why

    %w(strings go here)

is preferred to

     [strings, go, here]

The first form requires extra work to escape spaces, the second form is a representation of an array everywhere in Ruby, including in the debugger and IRB.

danenania3• 14 years ago

I'm sure this would only apply to an array with single word strings. If you need strings with spaces, %w definitely isn't appropriate.

masklinn2• 14 years ago

> The first form requires extra work to escape spaces

The first form is generally used for symbol-type strings (word lists, essentially, hence "w"). It is less syntactically noisy, shorter and requires less waffling around with non-word keys.

That's especially flagrant if you don't ignore half the actual array,

    %w(foo bar baz)

really results in

    ["foo", "bar", "baz"]

(the quotes are rather important, because they're noisy, repetitive and annoying to type).

It's one of the few things I regularly wish were in more languages.

Karunamon2• 14 years ago

Ahh.. I forgot about the quotes. That's what I get for posting immediately after waking up. Thanks :)

kreeger• 14 years ago

I can't stress enough how important it is to _not_ use `and` and `or`. As a former die-hard fan of those operators, it bit me in the ass so hard when those didn't exactly work the way they were supposed to. I've been using && and || now for years and I haven't looked back. /PSA

alanctgardner23• 14 years ago

To anyone who's wondering, it seems like 'and' and 'or' are the same as && and ||, but with lower precedence [0]. Does anyone have a good reason why they both exist? It's a pretty big gotcha.

[0] http://stackoverflow.com/questions/1426826/difference-betwee...

philwelch3• 14 years ago

It's an imported Perlism, back from the days of the old assert idiom `foo == bar or die`.

If `foo == bar` evaluates to true, the `or` is short-circuited, else it calls the built-in `die` function which kills the process.

gurkendoktor3• 14 years ago

&&/|| can bite you just as well:

if first_user = User.find(4) && second_user = User.find(6) ... end

look_lookatme• 14 years ago

Agreed. It's just not worth it, especially with implicit return.

sabat2• 14 years ago

They're not synonyms; "and" is not a more readable version of "&&".

As Avdi Grimm says: and and or, despite an apparent similarity to && and ||, have very different roles.

It's worth using "and" and "or", especially considering that && and || bind too tightly for some purposes. I try to avoid && and || unless I'm doing a specific boolean operation (e.g. x = foo || bar). Not for flow control, though.

If you really can't grok the logic, use parens.

Pewpewarrows2• 14 years ago

I agree. But in all code that I write, regardless of pretty much whatever language it's written in, I always explicitly add parentheses around what I want the order of operations to be.

"1 * 3 + 4 / 20 && foo" is unreadable. No one wants to memorize the order of operations for every language they use and have to figure it out in their head. Make life easier for the people who have to maintain that line of code you just wrote.

prawks2• 14 years ago

I can't believe more people don't do this. It seems like such a no-brainer when I'm writing. Not only for others, but for myself as well.

oinksoft• 14 years ago

For language-specific operators I could understand ... but to me, the cognitive load of the parentheses is worse than having to think for a moment about this particular set of operators which have the same precedence in every language I've used (and the arithmetic operators which I've had to know the order of since early grade school).

Maybe I'll eat my foot some day, but

  (a == 1) && (b == 2)

  (2 * 3) + 1

is nothing but code smell to me.

dpritchett• 14 years ago

I’ve learned a lot of programming languages over the years, and I’ve taken care never to learn the operator precedence rules in any of them. It’s easy to get them wrong and get bitten and why should I require that people reading my code learn those stupid rules.

- Tim Bray

[1] http://www.tbray.org/ongoing/When/201x/2010/06/29/No-Default...

graywh3• 14 years ago

Avdi Grimm gives a good explanation: http://devblog.avdi.org/2010/08/02/using-and-and-or-in-ruby/

danenania3• 14 years ago

Agree nearly 100%, except I see no problem with omitting parentheses around arguments in method definitions. I've never had any trouble parsing this and it's a bit cleaner to my eye.

OriginalSyn2• 14 years ago

In team environments you typically sacrifice convenience for maintainability and while you may not have a problem with optional parenthesis, it invites ambiguity and a more junior programmer may make a mistake because of it so the idea is to close off that edge case completely and not have to worry about it. This is the same reason a lot of shops ban the terniary operator for anything more than the simplest of variable assignments. It's not saying it's not useful, it's just that there are cases where if they had just been there in the first place coding errors could have been avoided.

danenania3• 14 years ago

I agree with your general point, but I don't really see how omitting those parentheses invites ambiguity.

def reply_to_post text

end

What could anyone imagine text to be in this context aside from an argument? I get that using parentheses is more familiar coming from languages with C syntax, but I see no other rationale. I've never worked with anyone who stumbled over this. It's always just: 'oh cool, I didn't realize you could omit parentheses there.'

yxhuvud2• 14 years ago

Agreed, unless the definition line spans multiple lines. Then ()'s are necessary for readability.

lucisferre2• 14 years ago

I think the issue is around the ambiguity and cognitive friction associated with figuring out where you require them and where you don't. Generally speaking I don't have a lot of problems with it, but I could see why someone might just say "always" to this one.

rauljara3• 14 years ago

There was one thing with block chaining that I was really hoping they'd hit on. They didn't. They had:

  # good
  names.select { |name| name.start_with?("S") }.map { |name| name.upcase }

I've noticed that sort of chaining is changing towards:

  names.select { |name| name.start_with?("S") }
       .map    { |name| name.upcase }

Which can be further chained (if need be) like so:

  names.select { |name| name.start_with?("S") }
       .map    { |name| name.upcase }
       .sort
       .join(', ')

Which, imo, is just better than trying to do it all on one line or using intermediate variables. It's more of a value call when it comes to the intermediate variables, though. I'd also use

  .map(&:upcase)

instead of

  .map { |word| word.upcase }

But I think it's perfectly understandable if you find the word.upcase more explicit and readable.

prawks2• 14 years ago

Agreed. I'm no rubyist, but it's not only much more readable, but functionally easier to manage in code. I can easily comment out one or more blocks or methods in the chain in this case if need be, for example. I'm surprised they advocate it on one line, actually.

edwinnathaniel• 14 years ago

Startin' to look like Java (with its fluent-interface pattern) :). Keep it up!

thumbs up

jordoh2• 14 years ago

This seems to be a popular style in javascript (particularly when using jQuery), but it's not actually valid ruby (edit: ruby 1.8.7, that is - see reply). The first line forms a valid statement, so the dots would need to be at the end of each line to indicate a line continuation. So it would need to be:

  names.select { |name| name.start_with?("S") }.
        map    { |name| name.upcase }.
        sort.
        join(', ')

Which is slightly more annoying in that you need to modify two lines to add a new chained line (the line you are adding and the dot added to the previous line).

This also creates a mild headache when refactoring, as any change to the length of the "names" variable would force you to re-align the entire chain. You could do something like:

  names.
    select { |name| name.start_with?("S") }.
    map    { |name| name.upcase }.
    sort.
    join(', ')

rauljara3• 14 years ago

> but it's not actually valid ruby.

You are correct that it is invalid for 1.8.6. The code runs just fine in 1.9, though. I take advantage of many things that don't work in 1.8.6, so I'm not particularly concerned with my method chaining breaking compatibility.

I prefer the more-than-2-spaces indentation because, by the shape of the code, it is clearer to me that this is a method chain and not a change in control structure. That is also obvious by actually reading the code, though, so I'm fine putting that down to personal preference.

pmahoney• 14 years ago

Regarding "def self.method" vs. "class << self; def method; end", one very big advantage of using the "class << self" technique (which Github discourages) is that it allows grepping for "def method" to find the location of the method definition.

Perhaps my editor is old and featureless (and Ruby's dynamic nature doesn't help), but I grep for such things multiple times per day.

dsr12• 14 years ago

Now-a-days we have computers which can show more than 80 characters at a time. "Keep lines fewer than 80 characters", Is this recommendation still valid?

barclay• 14 years ago

Yes(!) Goddamn you kids and your Xcode.

Now get off my damned lawn.

bryanlarsen2• 14 years ago

Shorter lines are easier to read. Short lines let you put up several editor windows side by side even on a cramped laptop. Short lines let you use a larger font to save your eyes.

There can also be a code win: when forced to deal with short lines, it's often easier to split a long line of code into two easier to understand lines than it is to split on an operator.

Sometimes a split line is a lot worse than a long line, which is why the rule shouldn't be applied religious -- you should only be called on it if you have many lines over the limit.

IMO, 80 is a little bit tight; I prefer 100. But I wouldn't bike shed it if there was a standard in place.

fox912• 14 years ago

I really can't stand longer lines, I think that this is a best practice for every code. I usually work with the monitor splitted in two parts with maybe 2 different files in each half, long lines would make that really difficult to read.

Moreover if you need lines longer than 80 chars you're probably doing it the wrong way: long lines are difficult to understand

olgeni• 14 years ago

> Now-a-days we have computers which can show more than 80 characters at a time

Of course we do. We also have serial ports that go way faster than 9600, so at last "ed" can be used to its full potential.

philwelch3• 14 years ago

I find 80 a bit cramped myself. There are a lot of lovely one-line idioms, like postfix if and iterator blocks, which feel constrained at 80 characters. Worse of all, I spend time worrying about line width.

In principle, having a line width limit is a worthwhile idea. 120 rarely gives me problems, and I can fit 240 characters wide in any case.

lucisferre2• 14 years ago

Lots of reasons to keep lines sort, though you could argue 80 is pushing it. I've found 100-120 is generally fine for most scenarios on modern hardware.

Just keep in mind that code will be read and shown in a variety of different places. For example reading the code on github or other source control tracking tools, reading it on different terminals with splits active (tmux and VIm splits), etc. Keeping it short just makes it more portable for readability purposes. That's definitely a good thing.

zxcdw• 14 years ago

This makes me want to learn Ruby so bad. :(

TommyDANGerous• 14 years ago

Love it. Great read.

onedognight• 14 years ago

I do not know of the single language, including the language of math, that gives equal precedence to their and and or operators. Sadly Ruby does, and it's a blight on the language. It's like giving equal precedence to + and \. It's a choice, but not one that anyone would expect.

What's worse, Ruby give the "correct" relative precedence to &&/||. I suspect this inconsistency is the reason the OP cautions against using and/or*. It's too bad because I miss the low precedence and/or operators (from perl) that can be used to avoid parenthesis that &&/|| would require in the presence of assignment, etc.

graywh3• 14 years ago

Ruby's and and or are for flow control like if and unless, not binary logic operators like (or synonyms for) && and ||.

jhund• 14 years ago

I use pretty much the same style for my Ruby work. One thing I do differently is replacing

  %w(word1 word2 word3)

with

  %w[word1 word2 word3]

both result in

  ["word1", "word2", "word3"]

I prefer the bracket delimiters since they better indicate the array nature than the parens. In % notation you can choose your own delimiters, so why not use brackets?

yxhuvud2• 14 years ago

Both are fine.

Just don't use linebreaks or weird utf8 characters, please.

EDIT: As delimeters. Feel free to use them inside the strings.

sabat2• 14 years ago

I like what I've read, but with a couple of cavaets. Ruby is the way it is on purpose. Discouraging people from using features of the language because the OP (presumably) does not understand how to use them: that's an anti-pattern.

The and and or keywords are banned. It's just not worth it. Always use && and || instead.

If you find "and" and "or" not to be "worth it", it tells me that you don't understand the difference. The "&&" and "||" versions bind more tightly, and that is not always desirable. Perl has the same set of constructs, and the Perl monks have been using them correctly for years.

As you can see all the classes in a class hierarchy actually share one class variable.

Well, yeah. That's meant to be a feature of the language, not "nasty behavior". It's good advice to use class instance variables, sure. But let's not imply that Ruby is a badly designed language; it's quite well-designed. This isn't Javascript. :-)

edit: formatting

bryanlarsen2• 14 years ago

Ruby is mostly well-designed, but it certainly has its warts. One of them is that 'or' and 'and' have the same precedence. They don't in Perl. That's one reason while Perl monks use them, but Ruby rockstars don't.

And please don't try to tell me that the lambda/proc/block mess is "well-designed".

Github Ruby Styleguide

Related Stories