-
-
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 |
@znvPredator how would concerns solve problems I've described? They would just move some code to other file
Yes, there will be more code. But it will be simpler.
The main problems I've described are:
- duplication
- confusing naming
- code is error-prone
- really complicated and fragile tests
@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.
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.
@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.
Why create 3 new objects, complicate the logic and basically make more code? I think refactoring it into concern will make code look much cleaner.