Fix read defaults after freeze #105

Merged
merged 6 commits into from Jul 21, 2012

Conversation

Projects
None yet
2 participants
@mbj
Collaborator

mbj commented Jul 19, 2012

No description provided.

Markus Schirp added some commits Jul 19, 2012

Markus Schirp
Add spec about frozen objects and reading defaults
* A fix will be in next commit to allow easy testing.
Markus Schirp
Ensure defaults are set prior to freeze objects
* As defaults are lazy-set on first access, accessing a
  attribute with a default on a previously frozen object
  blows up without this patch.

* I choose not to call #get_attributes as this creates an wasted
  attribute hash.
Markus Schirp
Handle exception differencies between 1.8 and 1.9
* This solution should also work for jruby, rbx etc.
lib/virtus/instance_methods.rb
+ #
+ def freeze
+ # Call get_attribute on all attribute to ensure defaults are set prior to freezing
+ attribute_set.each do |attribute|

This comment has been minimized.

Show comment Hide comment
@dkubb

dkubb Jul 20, 2012

Collaborator

I would suggest moving this into a private method named something like load_defaults or something clear. You can then move the comment to the method's summary.

@dkubb

dkubb Jul 20, 2012

Collaborator

I would suggest moving this into a private method named something like load_defaults or something clear. You can then move the comment to the method's summary.

+ include Virtus
+ attribute :name, String
+ end
+

This comment has been minimized.

Show comment Hide comment
@dkubb

dkubb Jul 20, 2012

Collaborator

Looks like the describe_class let blocks here and below have extraneous whitespace.

@dkubb

dkubb Jul 20, 2012

Collaborator

Looks like the describe_class let blocks here and below have extraneous whitespace.

This comment has been minimized.

Show comment Hide comment
@mbj

mbj Jul 20, 2012

Collaborator

Fixed. Thx. Takes time to rearrange my brain globally.

@mbj

mbj Jul 20, 2012

Collaborator

Fixed. Thx. Takes time to rearrange my brain globally.

This comment has been minimized.

Show comment Hide comment
@mbj

mbj Jul 20, 2012

Collaborator

@dkubb, lol in the first "Fix" it totally missed your point. Was only taking care of the whitespace after commas.

@mbj

mbj Jul 20, 2012

Collaborator

@dkubb, lol in the first "Fix" it totally missed your point. Was only taking care of the whitespace after commas.

dkubb added a commit that referenced this pull request Jul 21, 2012

@dkubb dkubb merged commit 0aa0649 into solnic:master Jul 21, 2012

@dkubb

This comment has been minimized.

Show comment Hide comment
@dkubb

dkubb Jul 21, 2012

Collaborator

@mbj this is fantastic, thanks, merged!

Collaborator

dkubb commented Jul 21, 2012

@mbj this is fantastic, thanks, merged!

@mbj mbj referenced this pull request Feb 5, 2013

Merged

Lazy default values #149

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