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

[Andrew Starks <andrew.starks@...>]

On Wed, Apr 16, 2014 at 10:50 AM, Rena <hyperhacker@gmail.com> wrote:
> 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.

As I mentioned in another post, another approach would be to provide a
way to express the intent of a value. That is, "this table is a
string" is something more than simply defining "__tostring."

Or, perhaps, it is the same thing and the fact that tables have a
default "__tostring" value makes "__tostring" far less useful than it
would be.

If "__tostring" could be reasonably interpreted as "Ahhh... I don't
have a table here, what I have is a string!", then libraries could use
your string-like table and you could adorn it with whatever puffery
you felt necessary and you could avoid the urge to patch the string
library.

But you can't so, there it is.

-Andrew