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

Add a PermissiveRespondTo extension for Mashes #499

Merged
merged 1 commit into from Nov 18, 2019

Conversation

michaelherold
Copy link
Member

By default, Mashes don't state that they respond to unset keys. This
causes unexpected behavior when you try to use a Mash with a
SimpleDelegator.

This new extension allows you create a permissive subclass of Mash that
will be fully compatible with SimpleDelegator and allow you to fully do
thunk-oriented programming with Mashes.

This comes with the trade-off of a ~19KB cache for each of these
subclasses and a ~20% performance penalty on any of those subclasses.

Closes #461

By default, Mashes don't state that they respond to unset keys. This
causes unexpected behavior when you try to use a Mash with a
SimpleDelegator.

This new extension allows you create a permissive subclass of Mash that
will be fully compatible with SimpleDelegator and allow you to fully do
thunk-oriented programming with Mashes.

This comes with the trade-off of a ~19KB cache for each of these
subclasses and a ~20% performance penalty on any of those subclasses.
@dblock
Copy link
Member

dblock commented Nov 18, 2019

@BobbyMcWho want to review this?

Copy link
Collaborator

@BobbyMcWho BobbyMcWho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes at the cost of approximately 20% performance for initialization and setters and 19KB of permanent memory growth for each such class that you create.

😬 Is this per class, or per instance?

Regardless, this looks useful in niche cases where someone really wants to use a Mash, I'd argue they should probably use a Dash unless they're just delegating methods that they don't have insight into whether they'd exist on the Mash.

LGTM

@michaelherold
Copy link
Member Author

Is this per class, or per instance?

The 19KB is per class. It's because each time you do include Hashie::Extensions::Mash::PermissiveRespondTo, we create a method cache on the singleton class of the includer. That's there to make sure we only have a 20% performance penalty instead of an order-of-magnitude penalty.

The performance penalty is only on the including class.

@dblock
Copy link
Member

dblock commented Nov 18, 2019

I am good with this, @BobbyMcWho merge at will.

@BobbyMcWho BobbyMcWho merged commit d5f2539 into hashie:master Nov 18, 2019
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jan 22, 2021
https://build.opensuse.org/request/show/865196
by user coolo + dimstar_suse
- updated to version 4.1.0
 see installed CHANGELOG.md
  ## [4.1.0] - 2020-02-01

  [4.1.0]: hashie/hashie@v4.0.0...v4.1.0

  ### Added

  * [#499](hashie/hashie#499): Add `Hashie::Extensions::Mash::PermissiveRespondTo` to make specific subclasses of Mash fully respond to messages for use with `SimpleDelegator` - [@michaelherold](https://github.com/michaelherold).

  ### Fixed

  * [#467](hashie/hashie#467): Fixed `DeepMerge#deep_merge` mutating nested values within the receiver - [@michaelherold](https://github.com/michaelherold).
  * [#505](hashie/hashie#505): Ensure that `Hashie::Array`s are not deconverted within `Hashie::Mash`es to make `Mash#dig` work properly - [@Michaelhero
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mash's respond_to? and method call do not behave in consistently
3 participants