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

Coerce improvements #96

Merged
merged 2 commits into from
May 10, 2013
Merged

Coerce improvements #96

merged 2 commits into from
May 10, 2013

Conversation

wapcaplet
Copy link
Contributor

Adjustments to make Coercion work better with Mash, per issue #95.

In `Hashie::Mash`, implement `#[]` and `#[]=` as `#custom_reader` and
`#custom_writer`, respectively, so they can be called explicitly
(aliases provided for backward compatibility). The `#deep_update` method
now uses the custom reader/writer methods, instead of the regular
reader/writer methods (which bypass coercion).

In `Coercion`, override `#custom_writer` to use the coercing setter
method. Additionally, force `.to_sym` on keys passed to `#key_coercion`,
to allow the string keys used by `Hashie::Mash` to work.

These together allow coercion to work with `Hashie::Mash` instances,
not only with direct `mash[:key]` symbolic indexing, but with `mash.key`
attribute style, `mash['key']` string indexing, and `Mash.new(:key =>
...)` instance initialization.

All spec tests are passing now.
@jch
Copy link
Contributor

jch commented May 9, 2013

Why did you choose to add alias :[] and :[]= to #custom_reader and #custom_writer instead overriding those methods in the module?

@wapcaplet
Copy link
Contributor Author

I tried using :[] and :[]= directly (i.e. self[key] = value in Mash#deep_update), but had some problems with infinite recursion. I never figured out exactly why, but the use of explicit #custom_reader and #custom_writer methods avoided the problem, and seemed to have a good symmetry with #regular_reader and #regular_writer.

@jch
Copy link
Contributor

jch commented May 10, 2013

@mbleigh any thoughts on this?

@mbleigh
Copy link
Contributor

mbleigh commented May 10, 2013

I originally abandoned implementing Mash et al because of infinite recursion problems, so it definitely happens. I don't think I have a problem with this if specs are passing.

jch added a commit that referenced this pull request May 10, 2013
@jch jch merged commit 961f718 into hashie:master May 10, 2013
@wapcaplet
Copy link
Contributor Author

Thanks for merging. If this needs revisiting sometime down the road, let me know--I admit this was kind of a hasty patch.

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

Successfully merging this pull request may close these issues.

None yet

3 participants