Things to avoid in Ruby | Hacker News Reader

Things to avoid in Ruby

blog.appsignal.com

56 pointsunripe_syntax68 comments

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.

There are two parts to Rubocop, one of which is good. The good part is the engine. The not so good part is the defaults, many of which are wrong and many more of which are "not even wrong" (which is to say that they are enforcing a particularly unidiomatic way of shaping the code based on unimaginative ideas of what good code is supposed to look like).

If you want to eliminate most discussions about code formatting and have a useful subset, use standardrb[1]. I disagree with about a third of its recommendations, but I stopped configuring with standard, whereas I had an override list at least thirty rules long (that had to be updated periodically because of incessant patch version renaming of rules) with vanilla Rubocop.

Rubocop's engine is good. Rubocop's default rules are best avoided and replaced entirely with standard, because they are at least mostly sensible and still allow for "style".

[1] https://github.com/standardrb/standard

That sounds like a team problem and not a tool problem. We do just fine with it (using rule configs that make sense) and uniformity isn’t the point. If somebody wants something changed for a legitimate reason then it is not a big deal.

You can also have different rules for different folders or files, and toggle rules on or off by line, block, or files.

The part about people spending significant time fixing linter errors sounds suspicious though. That doesn’t seem normal at all. Have they heard of format-on-save, or is there a lack of communication?

Rubocop is better than a free-for-all though. I'm more happy with a any config at all than people trying to rehash the same style of arguments in each PR. Or worse, ending in either a mess of formatting or each change changing unrelated lines just to reformat.

I really believe it's the least bad solution to just get any config and ignore the bikesheding (or drop people who spend too much time on it)

Theres a rubocop that disallows more than 5 or 6 function parameters. And since there’s nothing very lightweight like ADTs, people just move things to object state :(

Very sad.

If your team disagrees with this rule (or any other one), why wouldn't you just update the rule to match your expectations rather than dismiss the whole tool?

The problem is that is one example of MANY you will run into issues with.

I joined this codebase 10 years into it. We could turn it off now, but the damage is already done.

Interesting; I thought it does the opposite- remove the bike shedding talk.

It does sometimes recommend things that I don’t agree with sometimes, but in exchange I don’t have discussions about style on a PR, which makes it worth it. That’s objectively less time spent talking about these things.

I have a problem with the whole layout namespace of rubocop.

Then on top of that there are some rubocop extensions, the rspec one especially, that forces you to write bad, unreadable tests with the worst performance footprints. So unless you are in a company with a good culture where that talk does happen, I would keep rubocop for security sections.

In rspec it forces everything in a let, with single expect per test, so you can never check if two things are set to something at the same time

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.

The & thing can be handy, used sparingly, and the Rails issue of pulling results into memory to process them, rather than using 'pluck' etc is a real thing, but I agree the other suggestions are garbage.

I think this blog is called 'Ruby Magic', and Rails especially has that reputation of being inscrutable magic; these suggestions just make it worse. Forget idioms and using obscure features of the language or framework - just write simple clear code.

I’ve written a lot of ruby code but am not super advanced. Can you elaborate on what you mean about “pluck” and memory? Came across it in rails docs yesterday but haven’t seen it used in a codebase before

My guess is that they're referring to how Rails will instantiate full objects for ActiveRecord database requests and this is often not what you want, because we often just want a single or a few attributes from the model.

So if you had a model Activity, and you wanted to get the names of some of them, the slower/higher memory approach would be something like:

Activity.where("id < 100").map(&:name)

This pulls results from the database, instantiates each as Activity objects, then iterates through each to get its name. You can see in the logs that it grabs the entire object:

Activity Load (133.7ms) SELECT "activities".* FROM "activities" WHERE (id < 100)

.pluck instead grabs just the field you're looking for without instantiating the entire object. So the better way to grab the list of activity names would be like:

Activity.where("id < 100").pluck(:name)

And you can see from the logs that it makes a more narrow query:

(3.8ms) SELECT "activities"."name" FROM "activities" WHERE (id < 100)

Awesome thanks for the reply, am making an effort to learn more activerecord and didn’t know pluck would function like this. Was playing around with it last night on a ruby hash and missed the memory advantages

yes, this exactly, thanks.

I’m in a codebase now that dynamically generates functions, as if ruby/rails wasn’t hard enough to debug with its insistence on autoloading.

The whole stack is a dumpster fire filled with footguns.

Where “show me where this function is defined” turns into an impossible task.

mmmm this is a problem of the engineers not the language though =\

Overuse of dynamically generated functions is possible in any high level language. The worst offender that I had seen was a PHP wordpress plugin that stored a custom function per row in the DB brought to life via `eval`.

Those were the days.

> Overuse of dynamically generated functions is possible in any high level language.

A large number of high-level languages resolve functions at compile time, rejecting programs containing references to functions that don't exist.

No language in which dynamically generated functions exist does so, because if it did it would be impossible to use them.

I will agree that dynamically generated functions are not overused in languages in which they cannot be expressed at all, but...

After years of programming against REST(ish) APIs backed by statically typed languages, I'm now fighting one built by a team that loves ruby/rails.

It's a terrible experience. No auto-generated specs. Can't even look at the code to know what some request parameters are supposed to be (had to go digging through pagy docs for that one). One day, a value in a response that is normally a string was suddenly an array on certain queries.

The whole approach is so haphazard move-fast-nobody-cares that it grinds my gears and I'm not even contributing to their code, just consuming it.

My kingdom for a better experience.

Sounds like a bad API and not specifically Ruby issues.

The lack of openapi anything is a weakness of the ecosystem, as there's a ton of tooling that's left on the table. Even Python has a bit of a leg up here.

On top of that, the number of bugs that could have been avoided with static types is a bit depressing- the only true way to avoid them entirely is duplicating what you get from a static language, but manually with extra unit tests.

There's plenty of really nice things about Ruby and rails, but you don't actually get features faster without sacrificing something.

The team puts out code at more or less the same pace as I would have expected from teams working with other languages, though I haven't seen so many nil errors since working with a bad Java code base.

Just added open api to a rails API with rswag. It works by making the definition and then running your request specs against it. That certainly can make a lot of sense for your consumers

How is grep not solving this problem ? I've met this pattern across many codebases but a simple call to grep (or equivalent) usually answers those questions pretty quickly

grep doesn't help much when the codebase is full of subclasses that implement the same generically named method of some uber-super-class.

I assume (since such practice is fairly common in Ruby metaprogramming) that the methods are generated at runtime, so there is nothing to grep.

First, you cannot grep when the function is generated at runtime.

Second, what happens when the code is in a different repo and there are zero imports so you have no clue where it’s getting loaded from?

We’ve got multiple gems and microservices so I can’t even grep one codebase, I basically have to grep our entire gh org to find the function because ruby has terrible tooling to help with “go to definition”.

I've been in a situation at a job where we were trying to debug some code, not ruby, but still written very concisely and clever.

To even have a chance of debugging it we had to pull out more variables etc so we could breakpoint each part rather than it being this complicated one liner.

After they debugged it, and fixed it, I watched them rewrite it back into the complicated one liner because it looks nicer...

I just don't get it. It's like they just didn't see the pain they went through.

We built our own hells.

Was there some kind of side effect that prevented you from simply using the debugger to evaluate parts of the statement to interrogate the results?

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

I agree in general here, but safe navigation is almost always better, because you are reducing the amount of syntax being loaded into the developers head for the same effect, which helps to avoid getting bogged down and missing the bug, or losing sight of the overall goal. The level of terseness to employ is a balancing act and very nuanced, but safe navigation is pretty much a no brainer.

Regarding (3), I agree, after seven years on a massive Rails project working with tons of very talented Ruby developers, self#method= is typically avoided. However self#method is usually preferable -- when there's no assignment prior to reading the name, it's highly clear that the name is a method. There's always exceptions though.

Re (4), this is true but the decision of which calculation merits a temporary variable (assuming it's not needed for multiple use) can be pretty arbitrary. Elsewhere in this thread there is the sentiment that extra temporary variables increases debuggability which perplexes me, since most commonly Ruby debugging is done in a command line debugger like pry/byebug, and in that context you can simply evaluate part of the "packed" expression as needed. In languages which primarily use graphical debuggers and perhaps do not have the ability to evaluate arbitrary expressions, this is much more of a factor.

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)

Yeah, agreed; this whole article seems like good-hearted, but still from kind of a novice in Ruby: part of it is wrong and part is obvious

I agree -- the article recommendations are generally subjective/oversimplified. Except for point 5, which is a almost always a mistake of using the "tool at hand" (Ruby), and not knowing when to switch to a more appropriate layer (Active Record or SQL).

But what's wrong with an instantiated hash as kwargs? :)

It isn't a big win compared to just calling the method with it:

    meth(foo: 1, bar: 2)

    # even if multiline

    meth(
      foo: 1,
      bar: 2
    )

while needlessly allocating an hash (kwargs optimize that).

For this small example, I certainly agree.

But I like to use a prepared hash when there's preparation that must be done, or if it just makes the data more clear.

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}

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

This was my rationale originally, but after working on a big Rails codebase for so long, and hearing a lot of push for the opposite conclusion from Rails devs more experienced than I am, I have relaxed my opinion here. It is certainly true that always using self will avoid the type of bugs you speak of, but if you also aim for short declarative methods over long imperative ones (as you mentioned), this issue becomes pretty rare, and in a language with so many other ways to shoot yourself in the foot, really the only good way to insulate yourself against bugs is with automated tests.

What you gain from not using self is more compact expressions that are less likely to bleed past the line length limits or need to be split up into temporary variables, which does help to look at the big picture of an operation with less syntactic load on the reader of the code.

All that said, if you have junior Rails devs on your team, it might make sense to tailor your style closer to overuse of self to ensure they can more easily grok it.

Re (1, assignment of conditional), I tend to avoid this construction, though I don't stress about it too much in code review. It's just fairly easy to lose track of the fact that an assignment is occuring except in the most simplest of cases. I also extend this to implicit return -- like with explicit self I used to be militant that explicit return was always clearer and safer. However, in an "idiomatic" short method I think it's fine, you must understand and be cognizant of Ruby's return semantics no matter what. However, once it becomes clear that there is a chance someone will lose sight of the fact that we're doing a bigass return statement in this huge block of conditionals, I employ explicit return.

I'm also a huge fan of using early return to handle edge cases and exceptional cases. This is something a lot of developers disagree with me on, but I've never been sufficiently convinced to avoid it.

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.

For considerably long methods you are correct. But the rubonic ideal is shorter methods, and for such short methods, it doesn't provide much in the way of reducing cognitive load. As a reader, if you see no assignment prior to the access of the "prop", then it is a method. The problem comes when you can't fit the whole method on your screen, which is a pretty good rubric for whether you can fit the whole method in your head.

"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

I'm not sure what a filesort refers to here, but there are absolutely cases where :order can cause performance problems. If there is no index on the ordered field, you have to remember that the order applies first (ie before you filter the results) even though you might have written it last in your ActiveRecord chain.

However, the amount of times you want to order a subset of records independent of the complete set is quite rare, as almost always you are doing some kind of paged fetch, and it makes no sense to use an indeterminate database ordering, even if you are just ordering by primary key (which is always indexed in ActiveRecord)

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.

I'm with you about readability however the supposedly idiomatic way might be faster. I didn't run a benchmark but intuitively the more you stay inside the methods implemented in C inside the interpreter, the faster your code runs.

All in good measure. Too many loval temporary variables are quite annoying as well, especially paired with bad naming. When the names are like "thing_with_attr1", "thing_with_updated_attr1" and so on, it also becomes confusing which variable has what, when they are later used.

Having variable assignment after assignment after assingment is also an indication for a function potentially doing too many things and might even be called a code smell.

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

If Rubocop's default rules told you to jump off a bridge, would you? :-)

# rubocop:ignore JumpOffBridge

" (The attr_reader statement at the bottom "

Defining variables at the bottom of the class, WTF?

attr_reader defines reader methods that just return the ivar with the same name. They are just syntactic sugar, and sometimes it makes sense to put them after the private keyword, and private methods are idiomatically at the “bottom” of the class.

By the way, this is perhaps my biggest problem with Ruby. It is far too easy to have a lot of private methods and to lose sight of the fact that you're in the private section.

Things to avoid in Ruby: Ruby.

There are many things I like about Ruby, but almost all of them are in Groovy.

Ruby produced some of the best light-config or code-as-config software I've ever seen, and thankfully forced many aspects of Java's XML-SOAP-boilerplate problems to be finally addressed and thrown out.

But Groovy has static types and a legit JIT on the JVM with @CompileStatic. I agree very weakly with a lot of the contentions in the article because they are also major language advantages Groovy has (or had) over straight Java, but it is deck chair rearranging.

Ruby has had problems with performance and the usual problems with type-light scripting in large codebases. I don't know how good Ruby IDEs are, but fundamentally it has to be challenging to know what type and methods can be called on something being returned from a ruby expression, although it is amazing what Javascript IDEs can do with type inferencing so what do I know.

Groovy is a dead language though, killed by Kotlin, and Ruby is getting destroyed by Python.

A lot of Ruby devs moved to Elixir.

Is Elixir popular among Ruby fans or Rails fans? I really like ruby, kinda hate rails, and did not enjoy my one experience working in an Elixir codebase and haven’t managed to figure out why it keeps getting likened to Ruby.

I used Elixir for a few years in a Phoenix project, after and before many Rails projects and in parallel with a couple of Django ones. There are things I really like about Elixir and I wish I had them in Ruby. I never thought about using Elixir for everyday scripting, I'm using bash or Ruby for that, sometimes Python. Elixir is a big projects language for me, much like Java. Too much boilerplate for tiny things. Phoenix vs Rails, even level. Both trump Django from very far above.

23 year Rubyist here (and programming polyglot; I’ve shipped in 35+ languages or language dialects) who uses both Ruby and Elixir regularly.

I prefer to avoid Rails (it’s not bad, but it’s overkill for most things I need). The Elixir language has some superficial resemblance to Ruby which helped with language acquisition 8 years ago or so, but it is very much its own thing. The resemblance is unsurprising given José Valim's background with Ruby and Rails itself.

Most of my quick scripts end up being Ruby, but I prefer to think in Elixir (mostly for pattern matching) or Typescript (for structural typing). The libraries I maintain in the Ruby community support much older EOL versions of Ruby, so I haven't been able to adopt new syntax like pattern matching for much.

Ruby is fine for small projects by a single developer, anything that you could reasonably do a total rewrite in a day or two. Because the main way of refactoring is via rewrites.

As a language it is completely unsuitable for large projects, the dynamic features that are helpful at a small scale become liabilities at a large scale.

Ruby’s main flaw is the excessive exuberance of its fans, you wouldn’t develop a large website in bash, you probably shouldn’t do it in Ruby either, but each of the languages still has its own charm and is useful for the things it was designed for.

I've worked on multiple large Ruby codebases with many engineers contributing (ex: meta). There are many large Ruby projects out there, your one-developer-language opinion is simply not supported by facts

> Because the main way of refactoring is via rewrites.

Not sure where you are getting this.

Things to avoid: Whatever programming language you are currently using.

This sort of "language/tool/framework is a dumpster fire" post is not helpful or curious. Every tool has its flaws and places where it excels. Painting with broad strokes ignores nuance and implies a lack of familiarity with why the tool gets used.

Besides, PHP exists (/s)

Well said, a lot of people just don't get it.

Reply on news.ycombinator.com