I noped out at "Rubocop makes things better". It seems I'm not the only one in these comments who has encountered rubocop config gatekeepers. It seems there's always one person who vigorously defends some stylistic choice by claiming "for performance", "for readability", etc. until the rest of the team gets tired of arguing about the color of the bike shed and moves on to doing the actual work.

Sometimes I encounter code and go "I think I know who wrote this", and I don't have any problem with that. That's fine. People should have some freedom in the way they write code. A lot of the things I've learned in programming came from saying "ugh, I don't like this", and then later going "oh, that actually makes a lot of sense, and I'm going to adopt it".

I doubt very much there is any real developer time savings attached to enforced subjective uniformity, and in the case of rubocop I would say that the amount of time I've seen people changing this and that to satisfy the linter when what they wrote read just fine, it's probably the opposite.

I take issue with several of these. "Idiomatic Ruby" shoehorn's dogma into the least dogmatic language.

Elsif? How about a case statement?

A lot of the idioms are great until you need to debug them. At that point you have to replace them with non-idioms because you can't even debug the code you wrote without changing it. Why give someone grief for starting with something debuggable?

I dislike the social pressure and "Rubocop's Karen-ing" used to inflict these idioms when they are a mixed bag, at best, and demonstrably worse in cases.

I've been writing Ruby for the past 5 years or so and I think it's safe to say that these are very much opinionated takes on Ruby. There is no _one right way_.

A critique: 1. Verbosity vs Idiomatic, to me I find there are times the verbose approach is actually easier to read and understand

2. Long Expressions to Detect nil - this seems a bit like a strawman, sure there are better ways than to check the presence of an object/method chain, but doing the safe operator can bite you in the leg by compacting the complexity

3. Overuse of self - I skimmed through the Rectangle class declaration... it reminds me of writing Java. I rarely see self#method= used

4. Collecting Results in Temporary Variables - I think this is debatable, there's value in condensing method implementations when it makes sense, but also having temporary variable assignment can lead to better readability

1. About Verbosity: that's not common nor idiomatic ruby. You'd much more often see a case statement there, and nowadays with pattern matching. It look like idiomatic python though...

2. About Long Expressions to Detect nil: rails devs have had Object#try fir many ears to avoid that, and &. syntax since ruby 2.3 (10 years?).

3. About Overuse of self, again, clearly the author hasn't done enough python... and that's hardly common, given the many options ruby gives you (ivar access, self-less implicit method calls...)

4. About Collecting Results in Temporary Variables, ruby has supported map/filter/reduce for more than 15 years. This was however quite common code to find years ago from ex-java devs...

5. About Sorting and Filtering in Memory, that's actually the only legitimate problem I can think of. However, I'd hardly consider that Ruby's fault, and more of a superpower of composition of abstractions (datasets to enumerables) which should obviously be handled with care (and perhaps linked against? Does ruboxop support smth of the kind?)

Actual thing I see many times and should be avoided: construction an hash of options first, then pass it as kwargs:

    opts = {
      foo: 1,
      bar: 2
    }
    meth(opts)

1. Verbosity

The recommended fix here is…needlessly verbose. Better would be:

  actor =
    case response
    when 1
      "Groucho"
    when 2
      "Chico"
    when 3
      "Harpo"
    else "Zeppo"
    end

or even:

    actors = {1=>"Groucho", 2=>"Chico", 3=>"Harpo"}
    actors.default = "Zeppo"
    actor = actors[response]

2. Safe navigation operator

Yes, use this for sure, this is good.

3. "Unnecessary" use of self

Use of self to refer to instance methods (the text here refers to "model attributes", but that's just a particular use of instance methods) is appropriate and often desirable, because failure to do so as the example demonstrates earlier can result in bugs if the code is ever changed to introduce a local variable of the same names as the instance method. Now, sure, generally I'd prefer to avoid doing that, and in a very short method (especially a one-line defining what is essentially a getter for a derived trait), sure, don't bother, its cleaner and easier to comprehend without additional noise and no one is going to change it to add a local variable without seeing the other use and fixing it. But, in general, self is clear and safer.

4. Collecting results in temp vars

Yes, avoid this by knowing and using the methods on Enumerable that are designed for this, as they clearly and concisely express what you want to do.

But also, remember the first rule about Verbosity, so avoid stuttering and superfluous parens.

  h.transform_values {_1 &. *2}

I do prefer always using self.attribute, it’s a little bit longer but there’s no ambiguity. Otherwise you would have to have all props in your head when reading the code.

"Sorting and filtering are best performed in the database, if possible, using ActiveRecord's tools."

this needs a big fat asterisk.

sometimes sorting in the database requires a filesort

if you know ahead of time roughly how many results are getting sorted in-memory it absolutely can be faster to do it in-memory than ask the database to do it

On Collecting Results in Temporary Variables -

His 'more idiomatic' code is both harder to read (arguably) and harder to debug. In his "bad" example, it is very easy to set a breakpoint at each important step of the function, or insert logging, or whatever. The enumerable approach yields fewer lines but it's really difficult to stop the code and examine where something went wrong. Not to mention it's a bunch of symbol soup gobbledygook that takes a lot more specific Ruby knowledge to decipher.

Personally I'm all for temp vars. When I have to come back to the code in 18 months or whatever I will thank myself for making it so easy on me.

Ironically Rubocops style guide recommends against chaining the safe navigation operator.

" (The attr_reader statement at the bottom "

Defining variables at the bottom of the class, WTF?

[flagged]

Things to avoid in Ruby: Ruby.