Modifying core classes in a gem

Question

I'm developing a Gem which I think will be useful for more people than just me. One issue I'm facing is that I need to merge nested hashes. I found this useful Gist to accomplish that, but now I'm wondering if it's okay to modify Hash# like this in a Gem?

I'm sure there's a community "standard" or best practices that either accepts or declines this sort of code, so I'm turning to SO for guidance.

Thank you.

edited my answer, maybe it's more helpful now. – Patrick Oscity Jul 11 '12 at 09:07 — Patrick Oscity, Jul 11 '12 at 09:07

Patrick Oscity · Accepted Answer · 2012-07-11T09:07:34.663

5

When in doubt, just subclass Hash and include the Module in your subclass. You should particularly do this, when you override methods or change the behavior dramatically.

But i don't see why you shouldn't just modify the Hash class. Rails, for example, extends Core classes rigorously and i've never heard someone complain. You might take a look at how Rails' activesupport extends core classes:

https://github.com/rails/rails/tree/master/activesupport/lib/active_support/core_ext

Just make sure not to break existing behavior, so the users of your gem will not experience unwanted side-effects.

edited Jul 11 '12 at 09:07

answered Jul 11 '12 at 08:54

Patrick Oscity

53,604
17
144
168

Good answer! I am hoping that this question might turn into a community wiki or something like that. I'm marking your answer as correct for now as I think your argument for why this is okay is as good as any. Especially since my gem is aimed at Rails. – Emil Ahlbäck Jul 11 '12 at 09:13
And thanks for the link. I noticed activesupport already includes deep_merge :) – Emil Ahlbäck Jul 11 '12 at 09:15
Nice :) I would have included that in my answer but only found `reverse_merge` – Patrick Oscity Jul 11 '12 at 09:16

score 3 · Answer 2 · answered Jul 11 '12 at 09:26

You should always have a good reason for extending core class. I'd say that deep merging is good enough reason. To add new Hash method for deep merging like Hash#deep_merge - why not.

But I don't believe it justifies modifying existing method - overriding Hash#merge to support deep merge. People expect this method to behave certain way and if it suddenly behaves differently after requiring your gem, they won't like it. If you absolutely must do it, describe it at the prominent place in your documentation.

UPDATE: just read the comments under the other answer. Yeah, best solution in this case is simply to include activesupport/lib/active_support/core_ext/hash/deep_merge :)

Modifying core classes in a gem

2 Answers2

Linked