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

why does trash exists? #188

Closed
gregory opened this issue Jul 7, 2014 · 3 comments · Fixed by #297
Closed

why does trash exists? #188

gregory opened this issue Jul 7, 2014 · 3 comments · Fixed by #297

Comments

@gregory
Copy link
Contributor

gregory commented Jul 7, 2014

"A Trash is a Dash that allows you to translate keys on initialization"

Why shouldn't we just include an extension for the key translation @mbleigh ?

class Person < Dash
  include Hashie::Extensions::Dash::PropretyTranslation

  property :id, transform_with: lambda { |v| v.to_i }
  property :created_at, from: :creation_date, with: lambda { |v| Time.parse(v) }
end 
@mbleigh
Copy link
Contributor

mbleigh commented Jul 7, 2014

There's some history here. Originally Hashie didn't have any mixins for its functionality, instead relying only on subclassing. At one point, my plan was for Hashie 2.0 to do away with the need for the subclassed hashes entirely, leaving them in for legacy and convenience but making them only the compilation of a few mixins. I ran into some bizarre circular calling issues and more or less abandoned that work, even though in-progress some of it had been committed to the master branch.

Since then, other maintainers have taken up the mantle and honestly I don't have a great idea of where anything in this project is right now. Suffice it to say that Trash may well be completely implemented by your snippet above (I'm not sure either way) but the reason it exists is because it was added to the library in the "pre-mixin" era.

@gregory
Copy link
Contributor Author

gregory commented Jul 7, 2014

That was 4years ago :) will see what i can do about that when i get some time! thx

@dblock
Copy link
Member

dblock commented Jul 8, 2014

@gregory If you replace the Trash implementation by the above, do all tests pass? Probably a good way to figure out whether we should kill it or not :)

michaelherold added a commit to michaelherold/hashie that referenced this issue Apr 26, 2015
Issue hashie#188 brought up the fact that a Trash is really just a Dash with
some extended behavior. Why don't we make that a little more explicit
and define its behavior as an extension?

*Follow Up*

If this is merged, I move that we deprecate Trash for removal in 4.0.
I think reducing the number of classes will help users better understand
what behavior they are including in their subclasses.

In the long run, a 4.0 release would look really good with all of the behavior
factored out into extension methods. As we continue down that path, we can
slowly deprecate all of the `*ash` classes with the 3.x releases.

What do the other maintainers think of this suggestion?

Fixes hashie#188
michaelherold added a commit to michaelherold/hashie that referenced this issue Apr 26, 2015
Issue hashie#188 brought up the fact that a Trash is really just a Dash with
some extended behavior. Why don't we make that a little more explicit
and define its behavior as an extension?

*Follow Up*

If this is merged, I move that we deprecate Trash for removal in 4.0.
I think reducing the number of classes will help users better understand
what behavior they are including in their subclasses.

In the long run, a 4.0 release would look really good with all of the behavior
factored out into extension methods. As we continue down that path, we can
slowly deprecate all of the `*ash` classes with the 3.x releases.

What do the other maintainers think of this suggestion?

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

Successfully merging a pull request may close this issue.

3 participants