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.

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

mattr/cattr (aka attribute accessors) to keep up with the times #258

Open
SandNerd opened this issue Oct 4, 2016 · 6 comments
Open

mattr/cattr (aka attribute accessors) to keep up with the times #258

SandNerd opened this issue Oct 4, 2016 · 6 comments
Labels

Comments

@SandNerd
Copy link

SandNerd commented Oct 4, 2016

ActiveSupport's version of mattr changed a bit. Specifically what throw my code off was that I used the facets version but with a block which ActiveSupport version now supports.

@ioquatix
Copy link
Contributor

Submit a PR?

@trans
Copy link
Member

trans commented Oct 26, 2016

@sahal2080 Thanks for submitting this.

First note that Facets and ActiveSupports cattr/mattr methods are a bit different and in some regards that might always be the case. Unfortunately that's just the nature of beast, so to speak. Sometimes Facets needs to be on the forefront, and it's up to ActiveSupport to take lessons from it -- it's not a one-way street. So for instance, Facets supports :x= symbols. So you can do,

cattr :x, :x=

Instead of,

cattr_accessor :x

On the other hand ActiveSupport has options for :instance_reader and :instance_accessor that Facets doesn't.

cattr :x, :instance_accessor => true

I have to give this some thought (more below).

As for supporting blocks for default values, I guess that would be okay. Looks like all we would need to do is add class_variable_set("@@#{sym}", yield) if block_given? to the cattr methods. I think this is kind of funny/ironic though. A long time time ago people suggested the same thing for attr itself, but Matz never bit. Then I came up with an even even better notation.

attr :x => default

That way you can set defaults for all attributes listed. Not just the same default for all of them. Unless there is something I've overlooked, I like that approach better (though it is still possible to support block notation for comparability). Unfortunately it collides with the :instance_* options. So I'll have to think about that a bit more. And no, Matz never bit on that either ;-)

@trans trans added the feature label Oct 26, 2016
@IssueHuntBot
Copy link

@0maxxam0 funded this issue with $10. Visit this issue on Issuehunt

@IssueHuntBot
Copy link

@BoostIO funded this issue with $10. Visit this issue on Issuehunt

@ioquatix
Copy link
Contributor

@BoostIO @0maxxam0 what is the problem you are having? I can probably help sort this out.

@ioquatix
Copy link
Contributor

@Rokt33r is this spam?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants