The Chainsaw Infanticide Logger Manuever

[Jim Weirich]

Austin Ziegler said:

I think the real problem is when this is done in released code. If
you're going to extend code, extend it cleanly -- IMO.

Well said.

It is probably worth having a public discussion on the meaning of "extend
cleanly".


--=20
-- Jim Weirich (e-mail address removed) http://onestepback.org

 

[Austin Ziegler]

[NOTE: This is an offshoot of Zed Shaw's rant on Chainsaw Infanticide.]

Austin Ziegler said:
Well said.
=20
It is probably worth having a public discussion on the meaning of
"extend cleanly".

Aye, so I'll start. If you're going to extend core or standard library
classes, you should:

1. Do so only at the user's request. Diff::LCS *can* extend String and
Array but does not do so by default. If you are going to extend by
default, then you must document it in a very loud tone of voice, as
it were.

2. Don't depend on extensions to the core or standard library classes if
you're working on a *library* of code for others to use. Subclass,
extend (with a module), or delegate if you absolutely must. The
predecessor to Diff::LCS (Algorithm:iff) added #map_with_index or
something similar to Array and depended on it. I don't think that
Diff::LCS does that.

Applications and application frameworks may have exemptions. This
sort of allows for 1.day.ago notation as in Rails.

3. If you absolutely must extend the core and depend on it in a library,
try to use names that don't interfere with others.
Transaction::Simple follows #1, but it also follows this.

-austin
--=20
Austin Ziegler * (e-mail address removed)
* Alternate: (e-mail address removed)

 

[David Brady]

Aye, so I'll start. If you're going to extend core or standard library
classes, you should:

1. Do so only at the user's request.

I disagree. requiring a library should extend stuff in the standard
library if that is proper behavior for the library. E.g. require 'foo'
might well inject a parsing ctor into String as String#to_foo, if it
made sense to do so. I don't see any difference between a library and a
framework extending Fixnum with #day as per Rails. I agree strongly
with documenting it in a very loud tone of voice, however.

1.1 Do not change existing behavior of external classes or modules.
1.1.1 If you reopen an external class and redefine an existing method,
should Ruby issue a warning? Is there a safe_level that will prevent a
library from touching external classes?
1.1.2 Before adding a new method to a class or method, consider testing
with respond_to? to see if it really is a new method, and raising a
warning if it isn't.

4. In a library, never change existing behavior of code outside the
library. If I have a set of unit tests for a class, and I require your
library, all of the unit tests for my class must still pass.

It would be really useful if there were an idiomatic way of testing the
Core, StdLib and any modules the library required. Then a sanity test
could be run against the library verifying that all is as it should be.
This wouldn't be a short unit test, but something akin to the exhaustive
setup test you can run after building Ruby. It would be useful before
*publishing* a library to validate its sanity. Perhaps if you propose
to publish a module called foo, you may put test_foo.rb in the same
directory, or in ../test/test_foo.rb, and for any modules you require
that also provide a test_*.rb module, the sanity checker would run them
as well. Finally, modules in the RAA or other major repositories could
have the sanity checker's output placed alongside, letting the user know
to what extent it has been tested.

-dB

 

[Alexandru Popescu]

#: Austin Ziegler changed the world a bit at a time by saying on 8/24/2005 10:31 PM :#

[NOTE: This is an offshoot of Zed Shaw's rant on Chainsaw Infanticide.]

It is probably worth having a public discussion on the meaning of
"extend cleanly".

Aye, so I'll start. If you're going to extend core or standard library
classes, you should:

1. Do so only at the user's request. Diff::LCS *can* extend String and
Array but does not do so by default. If you are going to extend by
default, then you must document it in a very loud tone of voice, as
it were.

2. Don't depend on extensions to the core or standard library classes if
you're working on a *library* of code for others to use. Subclass,
extend (with a module), or delegate if you absolutely must. The
predecessor to Diff::LCS (Algorithm:iff) added #map_with_index or
something similar to Array and depended on it. I don't think that
Diff::LCS does that.

Applications and application frameworks may have exemptions. This
sort of allows for 1.day.ago notation as in Rails.

3. If you absolutely must extend the core and depend on it in a library,
try to use names that don't interfere with others.
Transaction::Simple follows #1, but it also follows this.

-austin

I will add my outsider 2c opinions:

1/ extend only if you don't have any other means to do it (meaning you have tried: subclass, extend
with module or delegate)

2/ if extending than use non-conflicting names and document them

3/ if you reopen an external class and redefine existing methods: document this and offer an alias

These apply imo to all core, standard and frameworks. Applications would be nice to behave the same
but not mandatory. However keep in mind that one row of documentation may change a whole day for
somebody else. Respect the others!

:alex |.::the_mindstorm::.|

 

[Austin Ziegler]

I disagree. requiring a library should extend stuff in the standard
library if that is proper behavior for the library. E.g. require 'foo'
might well inject a parsing ctor into String as String#to_foo, if it
made sense to do so. I don't see any difference between a library and
a framework extending Fixnum with #day as per Rails. I agree strongly
with documenting it in a very loud tone of voice, however.

I'm arguing that it's generally inappropriate for a random library to
add #day to Fixnum or to require a library which does so. See, I *do*
see a big difference between a library and an application framework. It
is inappropriate for a random library (say, Diff::LCS) to add a new
method to String and Array (#diff, as well as others). However, if one
is programming a Rails or Nitro application, then one is dealing with
Rails or Nitro and are, effectively, extending a *static* environment
that you control. Adding #day to Fixnum when you're a library, on the
other hand, is modifying an environment that you *don't* control (and
shouldn't).

Sometimes, as in #to_foo, it may be appropriate, but that should
*probably* be done with:

require 'foo'
require 'foo/string'

Alternatively, just use "require 'foo/string'" and 'foo/string' requires
'foo'. This is, IIRC, what Diff::LCS does.

Like I said, I'm not as hard and fast on this, but as a general rule,
one should avoid modifying those unless the user of the *library*
requests it.

1.1 Do not change existing behavior of external classes or modules.

I would generally agree with this.

1.1.1 If you reopen an external class and redefine an existing method,
should Ruby issue a warning? Is there a safe_level that will prevent a
library from touching external classes?

Ruby does issue a warning, when run with -w. And AFAIK, there is no such
$SAFE level that won't also restrict certain behaviours severely.

1.1.2 Before adding a new method to a class or method, consider
testing with respond_to? to see if it really is a new method, and
raising a warning if it isn't.
Maybe.

4. In a library, never change existing behavior of code outside the
library. If I have a set of unit tests for a class, and I require your
library, all of the unit tests for my class must still pass.
Agreed.

It would be really useful if there were an idiomatic way of testing
the Core, StdLib and any modules the library required. Then a sanity
test could be run against the library verifying that all is as it
should be.

Ultimately, this should be the Rubicon.

-austin
--=20
Austin Ziegler * (e-mail address removed)
* Alternate: (e-mail address removed)

 

[Julian Leviston]

--Apple-Mail-1-589937372
Content-Transfer-Encoding: 7bit
Content-Type: text/plain;
charset=US-ASCII;
delsp=yes;
format=flowed

But therefore the converse is also true - it allows you to open
untrustworthy code and bash it into trustworthiness

Julian.

Me either. I thought Logger was your own code. I agree that you
shouldn't have to unit test a library that you trust.

If it's just Logger that's crap, then you can shrug and decide not
to trust the library and respond accordingly. But you're right,
that Ruby allows other programmers to open a library you trust and
bash it into untrustworthiness. That's a real problem that should
be considered.

--Apple-Mail-1-589937372--

 

[David Brady]

Sometimes, as in #to_foo, it may be appropriate, but that should
*probably* be done with:

require 'foo'
require 'foo/string'

I *REALLY* like this idiom!

require 'mymodule/standardmodule' # allow mymodule to make injections to
standardmodule.

Nifty!

-dB

 

[Austin Ziegler]

I *REALLY* like this idiom!
=20
require 'mymodule/standardmodule' # allow mymodule to make injections to
standardmodule.
=20
Nifty!

Thanks. I didn't invent it, but it seems the *best* way to solve what
I'd said re: extending cleanly.

If Rails wanted to do this (as a library, not a framework), then it
might be require 'activerecord/date/fixnum' or something like that.
But that's just what *I* would do, and not what everyone would do.

-austin
--=20
Austin Ziegler * (e-mail address removed)
* Alternate: (e-mail address removed)

 

[Gavin Kistner]

So, you would then have a front-row seat if something was affecting
the logger output.

... which would lead you on the 3 hour hunt that Zed was complaining
about originally, to finally find the problem was someone else's
library.

You'd know just about as instantly as Zed did that something was
wrong. And you'd still have no idea what was causing the bug, or how
to fix it.

Unit tests are great, but I fail to see how they would have undone
ANY of the harm that the original library did, or how they would have
made Zed's life any easier in this one case.

 

[gabriele renzi]

Eric Hodel ha scritto:

The way silence_warnings is used is the real problem. Ruby already has
a perfectly good method of doing exactly what silence_warnings was
written to accomplish (two of them, in fact!).

talking about silence_warning in tests.. I actually found the need to
silence some warnings in test code recently and was thinking about
adding it myself.
What methods are already in ruby to get a "no warnign zone" in cases
where I know there are warnings that are not important, and still keep
em in the rest of the code?

 

[Trans]

Seems this thread just come over the fixed gateway? Seems to have
suddenly appeared with 34 post-fated messages in it. I've been missing
all the action!

Anyway, If I may add 2c. A good way to cleanly etend classes is via
AOP. Now granted you can still run in the same problems but there's
full SOC so they are much easier to track down. In fact using AOP would
be preferable to eve extending classes expect in those very basic
pretty much guarunteed safe cases --that is, if Ruby had tight AOP
support which it doesn't.

Matz has suggested the re, ost, :wrap hooks, and those will
ceratinly help albeit these are not full SOC since they still represent
a reopinging of a class.

T.

 

[Douglas Livingstone]

Glue even goes so far as to re-open the Logger class *just so it can turn=

it into a singleton*.

Could all these problems be solved by a require_into_namespace?

Say you have a library which modifies core classes. What if those
modifications existed only inside the library's namespace, so that
they don't leak into the rest of the application?

With a require_into_namespace (or better: require 'foo.rb', :namespace
=3D> 'bar') it would be the responsibility of the person doing the
require to put things in a namespace, rather than the original library
author, so you get your safety but keep your freedom.

Or is that totally impossible from an internals perspective?

Douglas

 

[Trans]

Douglas,

Matz brought the very ting up in his presentation on Ruby 2 some time
ago. So he's thinking about it, but I suspect it is hard to do. Also
I'm not sure if namespace is the appropriate term. Isn't "scope" the
better term?

But to answer your question. Yes, that would certainly help alot.
Although I can see the next argument now....

XYZ made all these great changes to ABC lib but hide it all in a
scopespace!
Come on! What good are all the changes if we can't use em ;-)

T.

 

[gabriele renzi]

Douglas Livingstone ha scritto:

Could all these problems be solved by a require_into_namespace?

IMHO this problems could be sloved by just running ruby by default with
$VERBOSE=true:
C:\Documents and Settings\gabriele>irb -r ubygems
irb(main):001:0> $VERBOSE=true
=> true
irb(main):002:0> require 'glue/logger'
c:/Program
Files/ruby/lib/ruby/gems/1.8/gems/glue-0.22.0/lib/glue/logger.rb:154:
warning: method redefined; discarding old format_message
=> true

 

[nlloyds]

One of the things that's really great about agile languages is they give you the power to do anything. One of the most horrible things about agile languages is they give every other idiot the same power to stab you in the back with a rusty pitchfork.

I needed to do some simple logging. I made a Logger class and started writing to it. Hmm, my log messages sometimes have a format and sometimes they don't. Nothing I do fixes this.

Oddly enough, current ActiveSupport does almost this exact same thing (https://github.com/rack/rack/issues/363) and last night I had a similar epiphany after a similar time sink that the original poster did 7 years ago.

Found this thread while googling and the title made me have to read it. Didn't realize how topical it would be. It's apparently going to be fixed in Rails 4, and there are known workarounds, but I just had to open this back up for the nostalgia.

Nathan