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

Don't monkeypatch Hash #3

Open
danleyden opened this issue May 16, 2017 · 1 comment
Open

Don't monkeypatch Hash #3

danleyden opened this issue May 16, 2017 · 1 comment

Comments

@danleyden
Copy link

fog-softlayer/lib/fog/softlayer/ext/hash.rb provides a monkeypatch on hash to add functionality to that class.

Adding a monkeypatch like that means that all users who include your gem end up having that method on the Hash object, which may interfere with their own overriding methods or interfere with similar things done in other libraries. This violates the principle of least surprise...

String is also monkeypatched, but in a less dangerous way. Because the additional methods have names that are very specific to this library, it is much less likely that they would interfere.

One possible way to fix this with minimal impact to the gem would be to use refinements which will only add the methods to the classes within the lexical scope where they are included (i.e. they wouldn't be used outside this gem unless somebody explicitly requests them)

@cphrmky
Copy link
Member

cphrmky commented Oct 25, 2017

Sorry to be so slow in replying to this. You make a good point, I'll make that update and fix this shortly.

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

No branches or pull requests

2 participants