Re: Monkey Patching is Bad, Unless Really Needed (was Re: A guide to building Lua modules)

On Wed, Apr 16, 2014 at 5:39 AM, steve donovan <steve.j.donovan@gmail.com> wrote:

On Mon, Apr 14, 2014 at 12:06 PM, Dirk Laurie <dirk.laurie@gmail.com> wrote:
> Rule #3: Allow monkeypatching
> Agree 5%. No monkeypatching should be the default, and
> it should be documented clearly when a module allows it.

There are a lot of good reasons for not patching Lua libraries, and
very few good reasons why one should. Quick (and temporary) fixing for
broken third-party libs is one of those good reasons.

If a person takes a dislike to table.insert[1] (and it's one of the
easiest functions to dislike, because it's really two functions in
one) then surely that person just defines a new function to do exactly
what they want? Why confuse the known and documented contract of an
existing standard library function?

And they may put it into 'table', this is a lesser evil and only
really applies to defining modules meant to be consumed by other
people's code. (stdlib used to do this a lot and fortunately Gary has
now made this optional)

As for routinely tweaking third-party modules, the same consideration
holds, since in a large system any global changes can have unintended
effects. That post I mentioned about why monkey-patching is bad for
Ruby gets the issue right: there are all these Rails plugins which
tweak the environment in incompatible ways, and - wait for it -
there's usually a perfectly good solution[2] that doesn't need such
radical surgery.

The reason for this rant: it is impossible to do big reliable projects
when people have this idea that they can patch things to suit
themselves. It's not the mechanism that bothers me, it's the policy;
the meme that patching is good engineering practice.

Perhaps there is some confusion between modules - stateless
collections of functions - and classes, where _instances_ have state?
Inheriting from a class creates a new class and one can do what one
likes[3] So at the least, make a copy of something that needs to be
patched, so that the patch doesn't propagate and mess with other vital
machinery.

Of course, this all applies to code that is meant to be used by other people ;)

steve d. (rant over)

[1] My favourite local assign is "local append = table.insert" for
purposes of appending at the end. It's possible to accidentally
trigger weird behaviour, but in practice I haven't seen this as a
problem worth getting excited about.
[2] http://devblog.avdi.org/2008/02/23/why-monkeypatching-is-destroying-ruby/
[3] modulo "fragile base class" problem. Inheritance isn't so cool
anymore, which is fortunate because the Lua community cannot agree on
how to implement it.

I had a thought about monkeypatching global tables:

#!/usr/bin/env lua5.2

do
    local _ENV = setmetatable({}, {__index=_G})

    local smeta = getmetatable('')
    smeta.__index = function(self, key)
        return _ENV._string and _ENV._string[key] or string[key]
    end

    _ENV._string = setmetatable({}, {__index=string})
    _ENV._string.mogrify = function(self) return self:reverse() end

    print(("this is a string"):mogrify())
end

print(("this is a string"):mogrify()) --this shouldn't work

It doesn't work as intended though, since the new __index has _ENV as an upvalue, so continues to look in it even when it's out of scope. If we could get the current scope's _ENV then I think it'd work.

--
Sent from my Game Boy.