Coercion problems with Hashie::Mash #95

Closed
wapcaplet opened this Issue May 8, 2013 · 3 comments

Comments

Projects
None yet
2 participants
@wapcaplet
Contributor

wapcaplet commented May 8, 2013

I noticed the Coercion extension recently added to hashie, and think it's a great idea. However, I've had some problems getting it to play nice with Hashie::Mash. For example, given these class definitions:

class User < Hashie::Mash
end

class Tweet < Hashie::Mash
  include Hashie::Extensions::Coercion
  coerce_key :user, User
end

If I create a new Tweet, passing a :user hash, coercion is not done; I end up with an embedded Tweet instance:

> tweet = Tweet.new(:msg => 'Hello', :user => {:email => 'foo@bar.com'})
 => #<Tweet msg="Hello" user=#<Tweet email="foo@bar.com">> 

Setting user via attribute reference (via Hashie::Mash#method_missing) does the same:

> tweet.user = {:email => 'foo@bar.com'}
 => {:email=>"foo@bar.com"} 
> tweet
 => #<Tweet msg="Hello" user=#<Tweet email="foo@bar.com">>

As does string-indexing:

> tweet['user'] = {:email => 'foo@bar.com'}
 => {:email=>"foo@bar.com"}
> tweet
 => #<Tweet msg="Hello" user=#<Tweet email="foo@bar.com">>

However, with symbolic indexing, coercion does work as expected, and the embedded user object becomes a User instance:

> tweet[:user] = {:email => 'foo@bar.com'}
 => {:email=>"foo@bar.com"} 
> tweet
 => #<Tweet msg="Hello" user=#<User email="foo@bar.com">>

I think it would be awesome if coercion worked with the other key-indexing methods (attribute, string, and initialize-based attributes) in addition to symbolic-key indexing, but I do wonder if there's a technical or compatibility reason this is not already the case. Is there?

If not, would there be any objection to me implementing this and submitting a pull request?

@wapcaplet

This comment has been minimized.

Show comment Hide comment
@wapcaplet

wapcaplet May 8, 2013

Contributor

I discovered that I can work around part of this limitation by coercing both the string and symbol, i.e.:

coerce_key :user, 'user', User  

This allows tweet.user, tweet[:user], and tweet['user'] to all be coerced properly, but Tweet.new(:user => ...) still doesn't do coercion.

Contributor

wapcaplet commented May 8, 2013

I discovered that I can work around part of this limitation by coercing both the string and symbol, i.e.:

coerce_key :user, 'user', User  

This allows tweet.user, tweet[:user], and tweet['user'] to all be coerced properly, but Tweet.new(:user => ...) still doesn't do coercion.

@wapcaplet

This comment has been minimized.

Show comment Hide comment
@wapcaplet

wapcaplet May 9, 2013

Contributor

I managed to make these use cases work with relatively few changes to Mash and Coercion. Please consider merging this pull request, and let me know if any further changes are needed to maintain backward compatibility.

Contributor

wapcaplet commented May 9, 2013

I managed to make these use cases work with relatively few changes to Mash and Coercion. Please consider merging this pull request, and let me know if any further changes are needed to maintain backward compatibility.

@jch

This comment has been minimized.

Show comment Hide comment
@jch

jch May 9, 2013

Contributor

Thanks for the pull. It's been a busy week, but I'll try to take a look
this weekend ;)

On Thursday, May 9, 2013, Eric Pierce wrote:

I managed to make these use cases work with relatively few changes to Mash
and Coercion. Please consider merging this pull requesthttps://github.com/intridea/hashie/pull/96,
and let me know if any further changes are needed to maintain backward
compatibility.


Reply to this email directly or view it on GitHubhttps://github.com/intridea/hashie/issues/95#issuecomment-17680271
.

-Jerry
@whatcodecraves http://twitter.com/whatcodecraves
github http://github.com/jch

Contributor

jch commented May 9, 2013

Thanks for the pull. It's been a busy week, but I'll try to take a look
this weekend ;)

On Thursday, May 9, 2013, Eric Pierce wrote:

I managed to make these use cases work with relatively few changes to Mash
and Coercion. Please consider merging this pull requesthttps://github.com/intridea/hashie/pull/96,
and let me know if any further changes are needed to maintain backward
compatibility.


Reply to this email directly or view it on GitHubhttps://github.com/intridea/hashie/issues/95#issuecomment-17680271
.

-Jerry
@whatcodecraves http://twitter.com/whatcodecraves
github http://github.com/jch

@jch jch closed this May 14, 2013

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