DEV Community

Cassidy
Cassidy

Posted on • Originally published at cassidy.codes on

Don't Trust the Cops: Sometimes Rubocop is Wrong

My team at work recently upgraded our version of Rubocop, the popular linter used to enforce good Ruby code style. With the upgrade we got a whole bunch of new suggestions and warnings about style violations.

One of them that tripped us up was the Performance/Count rule.

According to the Rubocop docs:

This cop is used to identify usages of count on an Enumerable that follow calls to select or reject. Querying logic can instead be passed to the count call.

So in plain Ruby, the cop sees that you are filtering an array with select or reject and then calling count on it. This is inefficient because count can actually do all this work for you.

But! What if you're in a Rails project and you have some code that looks like this?

@users.select { |u| u.admin? }.count
Enter fullscreen mode Exit fullscreen mode

Rubocop will autocorrect this to:

@users.count { |u| u.admin? }
Enter fullscreen mode Exit fullscreen mode

Looks okay? Well, this will likely execute a SQL count that ignores the block!!! Rubocop incorrectly assumes that @users is an Array. Why else would you be calling select on it, right?

In our case, though, @users was actually an ActiveRecord object that hadn't been loaded yet! Since ActiveRecord lazy-loads things, sending count to this object executes an SQL COUNT that ignores the block passed in!

Here's what it looks like in action:

User.count { |u| u.admin? }
# (0.4ms)  SELECT COUNT(*) FROM "users"
# => 320
User.count
# (0.6ms)  SELECT COUNT(*) FROM "users"
# => 320
Enter fullscreen mode Exit fullscreen mode

See how we got the same number each time here? This is because ActiveRecord ignores the block passed into count without raising an error and counts all users!.

The correct thing to do here is to pass your filter parameters into a where and then do a count.

User.where(type: 'admin').count
# (0.3ms)  SELECT COUNT(*) FROM "users" WHERE "users"."type" = $1  [["type", "Admin"]]
# => 10
Enter fullscreen mode Exit fullscreen mode

TL;DR

Top comments (0)