Skip to content

Instantly share code, notes, and snippets.

@mileszs
Created February 4, 2013 20:36
Show Gist options
  • Select an option

  • Save mileszs/4709536 to your computer and use it in GitHub Desktop.

Select an option

Save mileszs/4709536 to your computer and use it in GitHub Desktop.
Each document instance has categories and pages that are generated from "master categories" and "master pages". I think a refactor is in order. I would like to compare my result with the thoughts of others. How would you refactor this code (if you would refactor it)?
class Category < ActiveRecord::Base
belongs_to :document
belongs_to :master_category
has_many :pages
def generate_pages!
if pages.empty?
master_category.master_pages.each do |master_page|
pages.create!(master_page.page_attributes)
end
end
self.save!
end
end
class Document < ActiveRecord::Base
has_many :categories
has_many :master_categories, through: :categories
has_many :pages, through: :categories
def generate_categories!
MasterCategory.all.each do |master_category|
self.categories.create!(master_category.category_attributes)
end
end
def generate_pages!
self.categories.each do |category|
category.generate_pages!
end
end
end
class MasterCategory < ActiveRecord::Base
has_many :categories
has_many :master_pages
def category_attributes
self.attributes.slice('position', 'name').merge(master_category_id: self.id)
end
end
class MasterPage < ActiveRecord::Base
belongs_to :master_category
has_many :pages
def page_attributes
self.attributes.slice('position', 'name', 'index_page').merge(master_page_id: self.id)
end
end
@shipstar

shipstar commented Feb 4, 2013

Copy link
Copy Markdown

Same with generate_pages!, and I might move the save inside the if. That way, if you've made other changes to the object, you need to deal with those independently rather than relying on the accidental behavior that generate_pages! saves even if no pages change.

@unixmonkey

Copy link
Copy Markdown

Needs more transaction blocks wrapped around those each loops. Are these generate methods called called over and over and need to be idempotent (first_or_create)?

@mileszs

mileszs commented Feb 5, 2013

Copy link
Copy Markdown
Author

Thanks, guys!

They are called once per document, in the controller, during a wizard-esque process for creating a document.

I ended up making CategoriesGenerator and PagesGenerator classes that do all of the work. I'll post those soon, with some explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment