Skip to content

Instantly share code, notes, and snippets.

@windock
Last active September 1, 2015 21:42
Show Gist options
  • Save windock/dc13da62ba432f421ae2 to your computer and use it in GitHub Desktop.
Save windock/dc13da62ba432f421ae2 to your computer and use it in GitHub Desktop.
# why?
# Product is simpler - works with persistence only
# names in Product directly correspond to database (product_type => type, own_videos => videos)
# less error-prone - much more difficult to call wrong method. If anyone calls product_type instead of type, he will get wrong result
# a lot less duplication
# tests will be much more simple and fast
class Product < ActiveRecord::Base
has_many :videos # not own_videos - no need to have confusing names
def self.cast_collection
all.map(&:cast)
end
def cast
if parent_product?
self
else
ChildProduct.new(self, parent_product)
end
end
end
class ChildProduct
def initialize(record, parent)
@record = record
@parent = parent
end
delegate to: :parent # just meta code, may be implemented in a lot of ways
def videos
if inherit_videos?
@parent.videos
else
@record.videos
end
end
end
describe ChildProduct do
# if we use stubs, this collection of tests may be just generated, no database would be needed
describe '#videos' do
it 'delegates to parent if inherits'
it 'delegates to record if does not inherit'
end
end
@windock
Copy link
Author

windock commented Sep 1, 2015

@znvPredator how would concerns solve problems I've described? They would just move some code to other file

@windock
Copy link
Author

windock commented Sep 1, 2015

Yes, there will be more code. But it will be simpler.
The main problems I've described are:

  1. duplication
  2. confusing naming
  3. code is error-prone
  4. really complicated and fragile tests

@dmitry-solomadin
Copy link

@windock disregard my comment about concerns, I did not understand what you are trying to achieve here at first.

Ok, so in your example if I call videos on products it will go inside that method, return ChildProduct, suppose it doesn't inherit videos then it will delegate call to record and it will cause a inf. loop. Correct me if I'm wrong here but you still have to call either your association or your method differently.

@dmitry-solomadin
Copy link

Maybe we could use something like:

def product_type_with_inherited
  inherit_type? ? parent_product.product_type : product_type_without_inherited
end
alias_method_chain :product_type, :inherited

that's all the code we need for product_type association, here we are not breaking anything, can always safely call association by the name we want it to have and be sure inheritance will be taken into consideration, can easily access non-inherited attribute, so really everything you listed is gone. The only problem that I have with it is wired naming, but I think we can fix it too.

@windock
Copy link
Author

windock commented Sep 1, 2015

@znvPredator
Yeah, alias_method_chain will make this a lot more bearable. That's a great improvement over original code.
I have also greatly simplified my code, but yes, I'd prefer your solution for now.

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