This is an article for Code Polaris Advent Calendar 2020! !! !! !! My name is @kaori_cho and I have been working for Rails for the second year. I struggled with the code I was calling back this year, so I wrote to share what I learned there.
--Callback, implementation 1 second, debugging life --What you can do with callback can be done without callback --In the callback, write only the processing that can be executed infinitely.
In conclusion, be careful when writing callbacks! It is a story. Especially when developing a team.
In this article, for those who haven't written a callback yet, I'll explain the origins of anti-patterns for readability. For those who have already been addicted to it, what should I do? I hope it will give you an opportunity to think about it together!
Active Record callback https://railsguides.jp/active_record_callbacks.html
The quick story is that you can fire a specific process before and after creating or updating data.
Not written in code as callback
class User < ApplicationRecord
before_save :hogemethod
after_update :fugamethod
...
end
The one described as is a callback.
It is a function of Active Record and can be used in classes that inherit ActiveRecord :: Base.
I have no intention of saying that the callback is bad or not cool, and I think it is a powerful weapon if used properly. ** callback, it's so easy to put in, it doesn't look like it affects the whole application, but it's very effective ** For example, if the code grows like this, it will naturally become an anti-pattern? I will write and explain an example of.
There is a suitable model called User. Suppose a User has an id, name, email address, and birthday.
class User < ApplicationRecord
end
Suppose there is a common user information update process. Here, it is assumed that "a log remains when the user information is updated".
... that appears on the way as (omitted)!class UsersController < ApplicationController
...
def update
@user = User.find(current_user.id)
@user.assign_attributes(user_params)
@user.save
save_user_log #I'll leave a log!
redirect_to :back
end
private
#I'll keep a log when the user updates
def save_user_log
message = "#{@user.name}Information has been updated!"
UserLog.create(user_id: @user.id, log_message: message)
end
def user_params
params.require(:user).permit(:name, :email, :birthdate)
end
The log will remain when you update. Yes.
select * from user_logs;
+----+---------+------------------------------------------------+---------------------+---------------------+
| id | user_id | log_message | created_at | updated_at |
+----+---------+------------------------------------------------+---------------------+---------------------+
| 1 | 4 |Information about pikachu has been updated!| 2020-12-20 10:12:17 | 2020-12-20 10:12:17 |
+----+---------+------------------------------------------------+---------------------+---------------------+
Let's write the above process with callback.
It seems that after_update can be used to" keep a log when updated ".
** Callback is described on the model side instead of the controller. ** This is a super important point. (I'm addicted to it later)
Add to User model.
class User < ApplicationRecord
...
after_update :save_user_log
private
def save_user_log
message = "#{self.name}Information has been updated!"
UserLog.create(user_id: self.id, log_message: message)
end
end
So, the controller side will be like this There is no processing for save_user_log at all. This is also an important point!
class UsersController < ApplicationController
...
#Update user account information
def update
@user = User.find(current_user.id)
@user.assign_attributes(user_params)
@user.save
redirect_to :back
end
...
end
When I update it, the log is saved properly.
mysql> select * from user_logs;
+----+---------+------------------------------------------------+---------------------+---------------------+
| id | user_id | log_message | created_at | updated_at |
+----+---------+------------------------------------------------+---------------------+---------------------+
| 6 | 4 |raichu's information has been updated!| 2020-12-20 10:47:08 | 2020-12-20 10:47:08 |
+----+---------+------------------------------------------------+---------------------+---------------------+
Yeah, it looks good ~, callback! I was able to write it safely with callback. It was good. I'm smarter again.
By the way, the application will grow, so one day I decided to add such a function. "I want you to add an age column to the user table" Only birthdays can be entered from the outside, and age seems to be saved as data.
It's not so difficult, so let's ask Mr. C, who was in charge of implementing A and B. Mr. C added the age column, calculated the age from the birthday, and modified the controller to update.
class UsersController < ApplicationController
...
#Update user account information
def update
@user = User.find(current_user.id)
@user.assign_attributes(user_params)
set_age #I want to calculate and set my age from my birthday
@user.save
redirect_to :back
end
private
def set_age
if @user.age.nil? || @user.birthdate_changed?
birthday = Date.parse(@user.birthdate)
age = (Date.today.strftime('%Y%m%d').to_i - birthday.strftime('%Y%m%d').to_i) / 10000
@user.update(age: age)
end
end
def user_params
params.require(:user).permit(:name, :email, :birthdate)
end
end
I think it looks good. (For the sake of explanation, please pass through the processing that is not correct.) Let's update it.
Oh! You're getting older! It seems that the requirements can be met.
mysql> select id,name,birthdate,age from users where id=4;
+----+---------+------------+------+
| id | name | birthdate | age |
+----+---------+------------+------+
| 4 | pikachu | 1990-12-01 | 30 |
+----+---------+------------+------+
1 row in set (0.00 sec)
The feature has been released successfully. (No one knew what was happening behind the scenes ...)
Some time after the additional release of the age column, "The user_log is sometimes written twice, can you find out why?" I received a survey request.
Next time, let's ask Mr. D, who happens to be free. First, Mr. D goes to see user_logs.
mysql> select * from user_logs;
+----+---------+------------------------------------------------+---------------------+---------------------+
| id | user_id | log_message | created_at | updated_at |
+----+---------+------------------------------------------------+---------------------+---------------------+
| 4 | 4 |Information about pikachu has been updated!| 2020-12-20 14:19:15 | 2020-12-20 14:19:15 |
| 5 | 4 |Information about pikachu has been updated!| 2020-12-20 14:19:15 | 2020-12-20 14:19:15 |
Oh? Certainly, are they registered twice at the same timing? ?? ?? Moreover, it seems that there are times when it is registered twice and times when it is not ...
I can't help but go read the code. ..
Looking at the controller ... It's been a while since Mr. C fixed it, the code has grown, and various processes are in between.
class UsersController < ApplicationController
...
#Update user account information
def update
@user = User.find(current_user.id)
...
@user.assign_attributes(user_params)
...
...
Hoge.set_age(@user)
...
...
@user.save
redirect_to :back
end
private
def user_params
params.require(:user).permit(:name, :email, :birthdate)
end
end
Hmm ...? I wonder if the log is not saved here ...? Mr. D, who is in the first year of Rails, asks his senior for help and learns about the existence of callback.
And for some reason, look at the processing that was transferred to the Hoge class ...
class Hoge
...
def set_age
if @user.age.nil? || @user.birthdate_changed?
birthday = Date.parse(@user.birthdate)
age = (Date.today.strftime('%Y%m%d').to_i - birthday.strftime('%Y%m%d').to_i) / 10000
@user.update(age: age)
end
end
end
I think you already know who is good at Kang in the flow of this story, Actually, when the age is nil and when the birthday is updated, it becomes a double log.
Hoge class @ user.update (age: age) @ User.save in the UsersController class Then, update runs for user twice.
** So after_update: save_user_log in model has been called twice. ** **
(Since it says so, it is natural to say that)
I think there were some unnatural parts for the explanation, but what I wanted to convey in each sample code is as follows.
--A: Writing in callback can also be written without using callback in controller --B: Since callback is described in model, it is not clear what kind of processing is being done only on the controller side. --C: When adding a function, if you look only at the controller side, you may not notice the existence of callback (and it causes a bug) --D: Investigating when the callback is unintentionally ignited twice becomes more difficult.
In this way, it's convenient at first! Easy! It is a callback that can be implemented in, but as the code grows over time, it is often difficult to notice the existence of the callback. Of course, the above is the code for explanation, so honestly, I still understand. In fact, I went into the investigation from the standpoint of Mr. D and had a lot of trouble until I found a process that calls a callback in a process that is far away. .. It was like this.
――Clarification of conditions that are double-processed (I still can't consider what's wrong) ――Isn't it double transmission in API and asynchronous processing? (I haven't noticed the possibility of callback yet) --Since both user and controller are huge, it takes time to read the code (I start to think that it is a thin callback) --Since there are many other callbacks in the user class, even if it is a callback, I do not know who is the culprit (I doubt the callback) --Finally, I put in a monkey patch borrowed from a colleague, compare the methods called when it is duplicated and when it is not, with eye grep, and find the second update
Moreover, once this callback is embedded, it is extremely difficult to peel it off. I think that it may be almost impossible on a complicated system because you have to check every part of the application that is updating to user for side effects. (Actually, I did my best here as well, but since the number of characters and the deadline are all, I will take another opportunity ...!)
I knew I had a hard time. The important thing is what to do from now on. Here are some bullet points that I personally thought about.
--Write in callback only the processing that can be called more than once ――When there are 3 or more places where the same processing is already performed and the same processing is expected to increase at a constant pace in the future
Basically, if you can write something other than ** callback, don't write it with callback anymore. that's all. ** ** I wrote at the beginning that "a process that can be executed infinitely many times" because it loops infinitely if it has an impact and if the callback actually does not engage properly. Callback, the cost of writing is really low and it's impressive, but ** the cost of investigating and correcting something is too high. ** I would like to ask in the review, "Is it necessary to write it in callback?" For the more important business logic, especially the one that ignites only in one place.
--Write a test that the callback is executed only once --Match the recognition of callbacks as a team --First, educate students to read the code from callback
If you want to use it, I think you should focus on ** "Countermeasures against duplicate execution" and "Reduction of time cost for later people to read between lines" **. Callback is a nice and useful feature for allies. For example, even if it is okay for the log of this example to drop in double, if it ignites in double, such as "send notification email to customer" and "give points and coupons", implement the Akan thing with callback. Avoid. If you do it by all means, write a test to see the number of executions so that you will notice when it is executed twice. I wonder if this is all.
Thank you for reading this long article to the end! !! If you have any questions or concerns, feel free to contact @kaori_cho. I would appreciate it if you could tell me your mistakes secretly.
Well then, it was a difficult 2020, but I hope that next year will be another year of learning ~~~!
Recommended Posts