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.
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 bothconfig.eager_load_paths
andconfig.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.