I’d like to add a method to the String class for a gem I’m making.
Obviously, I could just open it up and pop the method in, but I’d like
to know what is the state-of-the-art for adding a method. Do I get
anything better from putting the method in a module and then opening up
the class and including it? I can’t see why but I’d like to know.
I don’t wish to use a new class that inherits from String because I
understand that there’s a gotcha with String due to something done for
performance reasons (though I can’t remember the exact details I
remember the warning!), and there’s obvious good sides to the method
being directly in String that far outweigh the inheritance angle.
I think there’s nothing wrong with monkeypatching (as long as you’re
adding methods, not overriding them).
If you want to be on the extra-safe side, you could just create a
module with the method and have the users of your library actually
include it in String. There should be noticeable performance
difference between the two ways.
On 19 Jun 2012, at 19:01, Bartosz Dziewoński wrote:
I think there’s nothing wrong with monkeypatching (as long as you’re
adding methods, not overriding them).
If you want to be on the extra-safe side, you could just create a
module with the method and have the users of your library actually
include it in String. There should be noticeable performance
difference between the two ways.
– Matma R.
Thanks for the reply. I’ll relax a bit now in the knowledge it’s not
just me. But then maybe it’s just us two!
I don’t have a problem with re-opening classes either. It’s a tool in
the Ruby tool box so why not use it. Especially when, as you pointed
out, inheritance sometimes doesn’t do what you’d expect. Some Ruby base
classes don’t call initialize which can be a problem.
Personally when I monkey patch I put my new methods in a module and then
include them in the target class.
module MyStringExtensions
def foo
“bar”
end
end
class String
include MyStringExtensions
end
This has three benefits;
Reuse. You can include the same monkey patch in different classes.
This almost never happens
Visibility. If you ask for the ancestors of the base class you will
see your module in the list which will give users a clue why the class
isn’t behaving in a standard way and a pointer to where they should
start looking.
Targeted extension. Using extend you can apply your monkey to
specific objects rather than every object of that type in the system.
Though, as @Bartosz pointed out, this may cause performance
degradation’s.
a = “asdfghj” # => “asdfghj”
b = “2345678” # => “2345678”
b.extend MyStringExtensions
a.foo #
NoMethodError: undefined method `foo’ for “asdfghj”:String
b.foo # => “bar”
Visibility. If you ask for the ancestors of the base class you will see your
module in the list which will give users a clue why the class isn’t behaving in a
standard way and a pointer to where they should start looking.
Targeted extension. Using extend you can apply your monkey to specific
objects rather than every object of that type in the system. Though, as @Bartosz
pointed out, this may cause performance degradation’s.
a = “asdfghj” # => “asdfghj”
b = “2345678” # => “2345678”
b.extend MyStringExtensions
a.foo # NoMethodError:
undefined method `foo’ for “asdfghj”:String
b.foo # => “bar”
I just learnt about this yesterday, and though it’s not applicable for
the situation I’ve got at the moment, it’s good to know, and this is the
first code example of it I’ve seen too.
Please do not monkey patch core libraries in gems, unless the purpose
of
your library is to add functionality to core, eg a library of extensions
like activesupport. Yes, this goes for additions as well as overrides. I
cannot tell you how many times I’ve seen a “harmless addition” mess
things
up in confusing and hard to debug ways.
Please do not monkey patch core libraries in gems, unless the purpose of your
library is to add functionality to core, eg a library of extensions like
activesupport. Yes, this goes for additions as well as overrides. I cannot tell
you how many times I’ve seen a “harmless addition” mess things up in confusing and
hard to debug ways.
While I understand it, I really disagree with this sentiment. Sure
monkey patching can cause problems, but so can a lot of things. With
great power comes great responsibility.
I really don't like this prevailing view that we should dumb down the
language to the level of the least skilled developers. Banning
unless/else and 'and/or', predicates returning true/false, inheritance
is bad, etc. This is the wrong approach.
Instead we should be raising the skills of the developers to enable them
to use the language to it's fullest.
I think the OP’s question has shown the right approach. He understands
the dangers so, in an effort to improve his skills, he has asked the
question, how can he use this technique in a way that will cause the
least amount of problems. We should be encouraging this.
Please do not monkey patch core libraries in gems, unless the purpose
of your library is to add functionality to core, eg a library of
extensions like activesupport. Yes, this goes for additions as well as
overrides. I cannot tell you how many times I’ve seen a “harmless
addition” mess things up in confusing and hard to debug ways.
What method do you want to add to strings?
Is there a way to limit the scope of an override or addition
to the namespace of the gem?
Here an example method that I use from time to time:
class String
def integer?
!!(self =~ /\A[±]?[0-9]+\Z/)
end
end
I think the OP’s question has shown the right approach. He understands the
dangers so, in an effort to improve his skills, he has asked the question,
how can he use this technique in a way that will cause the least amount of
problems. We should be encouraging this.
And I answered the OP’s question: he can cause the least amount of
problems
by confining monkey-patching to his own apps, not in publicly
distributed
gems. Or, if putting it in a gem, making it part of the publicly
advertised
feature set of the gem, not a hidden surprise.
Please do not monkey patch core libraries in gems, unless the purpose of your
library is to add functionality to core, eg a library of extensions like
activesupport. Yes, this goes for additions as well as overrides. I cannot tell
you how many times I’ve seen a “harmless addition” mess things up in confusing and
hard to debug ways.
Right, one should be very careful when messing with core classes.
While I understand it, I really disagree with this sentiment. Sure monkey
patching can cause problems, but so can a lot of things. With great power comes
great responsibility.
I really don’t like this prevailing view that we should dumb down the language
to the level of the least skilled developers. Banning unless/else and ‘and/or’,
predicates returning true/false, inheritance is bad, etc. This is the wrong
approach.
Instead we should be raising the skills of the developers to enable them to use
the language to it’s fullest.
I think the OP’s question has shown the right approach. He understands the
dangers so, in an effort to improve his skills, he has asked the question, how can
he use this technique in a way that will cause the least amount of problems. We
should be encouraging this.
I disagree: since OP only asked our support for applying a specific
technique but did not disclose any details about what functionality he
wants to add and why we cannot even judge whether the approach was a
good or bad fit for the problem he is trying to solve. Could be that
a simple method which receives a String argument was sufficient etc.
Ian, please disclose a bit more detail about what problem you are
trying to solve.
Ok, sorry to be the cause of confusion and/or consternation!
Let me preface this by saying that I do understand the dangers (I hope),
but that sometimes it is still appropriate so I wished to find out what
the 2012 view point is on the implementation. I take the view that it’s
hard to ask a stupid question, and it’s good to revisit things that you
feel are basics or “done” to improve or perhaps change (or just find out
you’re still right! Things change, and I don’t know everything
(/much). Having said that, I can understand why anyone would give a
strong “non!” to the idea without any specifics. I hope that was
diplomatic enough
I spliced together two libraries that deal with Postgres’ HStore
document format (a No SQL field, essentially) into a new one as an
update to a Sequel extension. I got it to pass the specs and do what I
wanted, put in the pull request, and the maintainer (among other things)
warned me that he’d prefer it if the core classes weren’t opened up.
This prompted the question, though I realise I can do this other ways
I’d also like to know the best way to do it this way - I was more
interested in the general point than the specific solution. The decision
to monkey patch was actually in one of the libraries I took from, I took
their ball and ran with it until the specs passed and then put in the
pull request. If it had been accepted then I’d have probably worked on
it some more.
If you’re still interested, the code in question is at https://github.com/yb66/sequel/blob/master/lib/sequel/extensions/pg_hstore.rb.
The method(s) would be a Hash#to_hstore and a String#from_hstore. When I
get a chance I’m going to make it into its own gem (on the advice of the
maintainer) and I’ll consider whether it’s a good idea to continue with
the monkey patches.
If you’re still interested, the code in question is at https://github.com/yb66/sequel/blob/master/lib/sequel/extensions/pg_hstore.rb. The
method(s) would be a Hash#to_hstore and a String#from_hstore. When I get a chance
I’m going to make it into its own gem (on the advice of the maintainer) and I’ll
consider whether it’s a good idea to continue with the monkey patches.
I my world String#from_hstore would be a class method at least. I
would also not cache @result in the String because that will prompt
all sorts of inconsistency issues when the String is changed.
Actually I’d rather have method HStore.from_string(str) and
HStore#to_string or maybe even HStore.from_db_string(str) and
HStore#to_db_string.
I am not sure I understand yet why you have three types involved:
String, Hash and HStore. You certainly need String and HStore. But I
would consider conversion to or from Hash just as a convenience
feature of HStore i.e. to easily create one. That would make changing
Hash superfluous. And all conversion methods could go into HStore as
explained above.
Btw, you can turn
token_pairs = token_pairs.map { |k,v|
[k,v].map { |t|
case t
into
token_pairs = token_pairs.map { |a|
a.map { |t|
case t
Things change, and I don’t know everything