I wanted to start refactoring my Redmine merge plugin now. I’m getting it ready for a release so doing a few refactorings now will help me make sure it’s working properly.
The Refactoring
Using inline temp I was able to clean up how the SourceNews#migrate
converts data from one database to the other database.
Before
1 2 3 4 5 6 7 8 9 10 11 12 |
class SourceNews 'SourceUser', :foreign_key => 'author_id' def self.migrate all.each do |source_news| n = News.new n.attributes = source_news.attributes n.project = Project.find(RedmineMerge::Mapper.get_new_project_id(source_news.project_id)) n.author = User.find_by_login(source_news.author.login) n.save! end end end |
After
1 2 3 4 5 6 7 8 9 10 11 |
class SourceNews 'SourceUser', :foreign_key => 'author_id' def self.migrate all.each do |source_news| News.create!(source_news.attributes) do |n| n.project = Project.find(RedmineMerge::Mapper.get_new_project_id(source_news.project_id)) n.author = User.find_by_login(source_news.author.login) end end end end |
Review
Since SourceNews#migrate
was a pretty simple method, this refactoring is easy to understand. By using the block method of ActiveRecord#create!
I don’t have to worry about calling #save!
at the end. It’s also a nice and simple way to keep all of the News
construction in places (inside the block).