Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Accessing attribute values from a hash rather than a reader method #113

Closed
jfelchner opened this issue Nov 7, 2012 · 6 comments · May be fixed by #122
Closed

Accessing attribute values from a hash rather than a reader method #113

jfelchner opened this issue Nov 7, 2012 · 6 comments · May be fixed by #122
Labels
Inconsistency

Comments

@jfelchner
Copy link

Currently I have a couple gems which utilize low level methods on A/R to make working with dates and times easier.

I override the high level readers/writers and use lower-level write/read_attribute so that I don't get stuck in a 'stack too deep' conundrum.

Once I found ActiveAttr, I was extremely happy because I had a few models which I had inherited from A/R::Base and all the baggage that comes with that and I was looking forward to using ActiveAttr instead.

Unfortunately, because ActiveAttr doesn't follow A/R's model more closely, my gem integrations break. Specifically, the problem is:

class MyActiveAttrClass
  include ActiveAttr::Attributes

  attribute :sweet_sauce

  def sweet_sauce
    puts "Yo, I'm gettin some sweet sauce!"

    read_attribute(:sweet_sauce)
  end
end

class MyActiveRecordClass < ActiveRecord::Base
  # This model has a 'sweet_sauce' column in the DB

  def sweet_sauce
    puts "Yo, I'm gettin some sweet sauce!"

    read_attribute(:sweet_sauce)
  end
end

my_attr_instance = MyActiveAttrClass.new
my_attr_instance.sweet_sauce = 'marinara'
my_attr_instance.sweet_sauce

#=> Yo, I'm gettin some sweet sauce!
#=> Yo, I'm gettin some sweet sauce!
#=> Yo, I'm gettin some sweet sauce!
#=> ...
# Finally blows up in 'stack level too deep'

my_record_instance = MyActiveRecordClass.new
my_record_instance.sweet_sauce = 'ranch'
my_record_instance.sweet_sauce

#=> Yo, I'm gettin some sweet sauce!
#=> 'ranch'

The reasoning behind this (as I'm sure you know) is that ActiveAttr delegates to the accessor method when read_attribute is called. The fallacy in this approach (in my mind) is that the top level accessor methods are the most likely to be overridden for different functionality. And in A/R it is expected that as long as you call read/write_attribute at the end of your overridden reader/writer, all DB functionality will work as expected.

I feel like the correct approach to be compatible with gems which are using lower-level A/R would be:

accessor => read/write_attribute => attributes hash on the instance

Thoughts?
@cgriego
Copy link
Owner

cgriego commented Nov 7, 2012

You might be right that we need to switch in order to achieve least-surprise with people converting from ActiveRecord.

For now you can workaround this surprising behavior by leveraging Ruby's standard method override behavior and calling super from within your override.

@jfelchner
Copy link
Author

@cgriego Thanks for the quick reply! Unfortunately calling super isn't going to work in my case for the same reason I can't create an accessor method and use super to call the accessor created by ActiveAttr. This is because both the accessor created by ActiveAttr and the accessor created by my gem are at the same inheritance level.

Fortunately I just did a spike and because you've already implemented an attributes hash on the instance in ActiveAttr::Attributes, replacing usages of the readers/writers with the hash seems to be trivial. I forked it, made some simple changes (5 lines) and got it to work with my gems.

I'll probably be able to submit a pull request in a few days.

@cgriego
Copy link
Owner

cgriego commented Nov 8, 2012

Without seeing your gem code it's hard to provide any guidance, but ActiveAttr does not create the accessors directly on the model class, it uses an anonymous module. That's how defining an override directly on the model and calling super can work. If you gem is not defining overrides on the model (which would work I would think) then maybe it too can use the anonymous module technique if dynamic, or if the overrides are static then just a plain module.

@jfelchner
Copy link
Author

@cgriego you just taught me something new. That's how my gems should be doing things as well. :) Thank you.

Nevertheless, as long as you are ok with it, I'll get this pull request to you. I still feel like accessing the attributes hash is a slightly better implementation.

On a side note, have you looked into actually modifying Rails to have some of this modularity?

@cgriego
Copy link
Owner

cgriego commented Nov 9, 2012

I am still interested in a pull request that changes ActiveAttr to work in a less surprising way.

By modularity, do you mean the use of the anonymous module or the way that ActiveAttr is broken into modules? Rails actually uses the anonymous module technique, ActiveAttr is leveraging the same code that ActiveRecord uses, ActiveModel::AttributeMethods. That may be useful for your gem.

@kainosnoema kainosnoema mentioned this issue Jul 6, 2013
Open
@jfelchner
Copy link
Author

Clearing out my "open" issues. This is obviously never going to happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Inconsistency
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Read/write attribute directly from @attributes to match ActiveModel
2 participants
@cgriego @jfelchner