Bad Rails Practices

by mmyoji

4 min read

This post is just my feeling about bad practices for Rails applications I have ever seen.

I hope this post will decrease such horrible apps and help people who have stress toward such Rails apps.

This might contains "Before discussing Rails" and my wish for developing Rails app.

No/Less Database Constraints

Sadly, this case is not a small number. I'm really sad and mad with this because DB is the last bastion of web app.

NOT NULL constraints must be added at least, and you should have both DB constraints and ActiveRecord validations as much as possible. (Of course, you don't need to add complicated constraints to DB.)

As for foreign keys, it is argumentative but you should use it if you don't have concrete opinion to it.

string enum is easy to change when you have to add a column for enum esp. in start-up project which often changes its specifications because integer enum is difficult to migrate data afterward. At least, you should specify the value and its integer value like as I write in following code. AR::Enum supports only integer column, you can use enumerize for string columns.

class Order < ApplicationRecord
  # difficult to modify
  enum status: %i(initial pending shipped complete canceled)

  # specify the value - number
  enum status: { initial: 0, pending: 1, shipped: 2, complete: 3, canceled: 4 }
  # or set more flexibility
  enum status: { initial: 0, pending: 1, shipped: 2, complete: 5, canceled: 10 }

  # more flexibility w/ string column
  extend Enumerize
  enumerize :status, in: %i(initial pending ...)
end

No/Less AR validations

In worse case, they write validations manually and don't use AR's validations... You should read Rails Guides at least before writing code on Rails.

However, conditional validations can be confusing and wrap the procedure as class or method.

By the way, you should avoid callbacks because it easily produces bugs.

Adding lib/ to autoload_paths

This depends on the architecture of a project, but Rails doesn't add lib/ to autoload_paths default.

As it says here:

But using autoload_paths on its own in the past (before Rails 5) developers might configure autoload_paths to add in extra locations (e.g. lib which used to be an autoload path list years ago, but no longer is). However this is now discouraged for most purposes, as it is likely to lead to production-only errors. It is possible to add new locations to both config.eager_load_paths and config.autoload_paths but use at your own risk.

It's a kind of Rails-way, I guess.

When code contains application specific logic, it can be put under app/. If not, publish as a rubygem and it will be a good PR for the project or the organization.

Less Transactions

Just use it, please.

Specify Gem's version with No Reason

They might not know Gemfile.lock. You don't need to specify the versions of gems in Gemfile if you have no reason to do so. Run bundle update periodically and you can find out problems in daily development process. (You need to have enough test coverage, though.)

Run bundle update everyday if possible and the diff of gems becomes less.

Create extra directories under app/

Though this also depends on the architecture, working with default directories might be the answer of Rails. If the extra ones are needed, the framework must have them.

As an exception, a layer which is introduced by gem can be accepted normally. But you don't need like app/workers and app/uploaders because the current Rails have ActiveJob and ActiveStorage as default.

I can just say you have app/decorators w/ draper or active_decorator. Working with helper is really hard when the app has some amount of views.

More create/save rather than create!/save!

This is not always true, but not least ones use save even when you must want the process is successfully operated.

Use methods with ! when you want to ensure success and catch exceptions.

Or at least, use if clause and check the result of action.

Many class methods

You just can say "study OOP" at this level.

The most problematic thing is adding a class method which issues find or where (though you can fetch data from relations) and it produces N+1.

Basically utilize AR's relations to make preload/eager_load/includes work properly, and define relations before doing it.

Soft deletion with No consideration

Unless the app has a feature to restore data, you don't need soft deletion in most cases.

You just prepare another DB when you need data for analytics, and can delete it after some period even it required.

Without that, garbage will increase as time goes by.

When you use RDS on AWS, it can create backups and you can rely on it when a trouble happens.

Issue DELETE directly than adding soft deletion with "in case".

Summary

I just complained.