Re: The Chainsaw Infanticide Logger Manuever
Dave Fayram
On Aug 24, 2005, at 10:55 AM, Jeff Wood wrote:
> Ok, my set of tests for my application would include asserting that if
> a given piece of functionality is going to cause a log message to
> occur, that I validate the residual log files do contain the log
> entries I expected as I expected them to be formatted. If they don't
> then the application/library doesn't meet it's spec and/or
> requirements. It's a bug and it needs to be resolved.
>
> So, you would then have a front-row seat if something was affecting
> the logger output.
At some point--and that point approaches us very quickly on anything
larger than a trivial project--we have to trust that our libraries
work as documented and that others won't deliberately mess our
environment up. Extension of a class to have a to_whatever is one
thing, changing behavior that was there before is another.
So, I'm not sure that testing this kind of thing is even reasonable.
It might be nice to have, but part of the point of TDD is to keep you
on track and developing at a consistent pace. If I stop to check every
method I might use, I'm going to get sidetracked far too quickly in
a language like Ruby
I think the moral of the story is that Logger needs a meta channel
which records the state of the logger itself. I've used loggers that
have such a feature and it's surprisingly useful.
> And no, I don't believe there is any difference between building a
> library and building an application ... it's all code that needs to be
> tested and made sure it functions completely as designed.
>
> I hope that answers you sufficiently.
>
> j.
>
>
> On 8/24/05, Bill Kelly <bi...@cts.com> wrote:
>
>> Jeff,
>>
>> From: "Jeff Wood" <jeff.da...@gmail.com>
>>
>>
>>> The truely iterative & testing-complete way to build software is to
>>> design and implement your tests before you write your code. When
>>> all
>>> of your tests pass, you're done...
>>>
>>> ... If you built your system like that, you should have tests for
>>> each
>>> and every action on your classes ... and the first time somebody
>>> elses
>>> modifications has an affect on your functionality and expected
>>> output,
>>> you should know about it.
>>>
>>
>> I appreciate the well-meaning nature of your replies, but I've
>> been programming for 24 years and doing TDD semi-consistently
>> for the last 5.
>>
>> Again, I'd ask to see what sort of test you'd propose that would
>> catch the problem I described. (My post, the one you quoted, was
>> about CGI, not the Logger.)
>>
>>
>>> To answer "how/why would you test this"... you should have tests in
>>> place for your logging functionality. The second something doesn't
>>> come out right, you know that something is marring the system.
>>> Debugging starts by walking the source of the packages you are using
>>> and watching for references to the modified functionality... It's
>>> not
>>> that hard to track down.
>>>
>>
>> Thanks for the tip. =D
>>
>>
>> I also would ask again, Do you not consider libraries and
>> applications
>> different, where Ruby's beloved Openness is concerned?
>>
>>
>> Regards,
>>
>> Bill
>>
>>
>>
>>
>>
>
>
> --
> "So long, and thanks for all the fish"
>
> Jeff Wood
>
>
--
Dave Fayram <dfa...@gmail.com>
Software Developer
Bill Kelly
Alexandru Popescu
> The truely iterative & testing-complete way to build software is to
> design and implement your tests before you write your code. When all
> of your tests pass, you're done...
>
> ... If you built your system like that, you should have tests for each
> and every action on your classes ... and the first time somebody elses
> modifications has an affect on your functionality and expected output,
> you should know about it.
>
>
Why running away from the idea of the original mail? Having the tests may solve the problem. Or they
may not: what if your tests are behaving correctly just because of the wrongly handled code outside
of your control? Should you start to develop tests for that lib too? No.
When you expect something from a lib you take it as documented. If that lib does something
undercover it should document it. It's simple. The tests have their power, but they are not the
silver bullet.
respectfully,
:alex |.::the_mindstorm::.|
> On 8/24/05, Bill Kelly <bi...@cts.com> wrote:
>> From: "Jeff Wood" <jeff.da...@gmail.com>
>> >
>> > You shouldn't be afraid of having power. That's why you have tests.
>> > You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
>>
>> Dudes, I do have tests. How in blazes would you write a test
>> to catch the problem I described? (Please show your work. :)
>> And why would you ever think to do so?
>>
>> It's not being afraid of having power. In my own applications,
>> I'll exploit any and every cool capability of Ruby I feel like.
>>
>> But when I personally write a module I view as a library I'd
>> like others to find useful, I take a decidedly more conservative
>> approach - deliberately.
Zed A. Shaw
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.
Three hours later and half my sanity, I start to snoop through the libraries I've included, and I find this wonderfully horrible gem:
http://rafb.net/paste/results/0vADxJ56.html
Not only does this code re-open a class to do something that could *just as easily be done with subclassing*, but the author also TURNS OFF DOCUMENTATION with :nodoc: so that nobody knows about it. Great! So much for Principle Of Least Surprise. This is more like Principle Of Most Heinous Arsenic Injection.
I then trolled through a few more projects and found that this is common practice. The glue project does it. Glue even goes so far as to re-open the Logger class *just so it can turn it into a singleton*. Great Buddha the insanity is everywhere!
http://rafb.net/paste/results/yXNJvv50.html
What's wrong with you people? This is the absolute worst thing I've ever experienced in any language I've used. I thought having magic AspectJ crap silently move my code was bad. At least that stuff was documented and I could optionally turn it off. This can only be fixed with leprechaun backflips into a pool of jello while covered in chocolate. Translation: it can't be fixed easily and I shouldn't have to "fix" it since I didn't break it.
-- Please, think of the children before you re-open their parent with a damn chainsaw. --
I only re-open a class after I've tried every other option like subclassing, wrapping, etc. If nothing else works or is too much effort for the modification, then I DOCUMENT THE HELL out of my modification and try desperately to localize the change so that it doesn't hurt anyone else.
Alright, rant off. Please stop doing this. My appologies if I hurt anyone's feelings.
Zed A. Shaw
http://www.zedshaw.com/
Daniel Berger
> So,
> To answer "how/why would you test this"... you should have tests in
> place for your logging functionality. The second something doesn't
> come out right, you know that something is marring the system.
> Debugging starts by walking the source of the packages you are using
> and watching for references to the modified functionality... It's not
> that hard to track down.
As Alexander mentioned, the point of the email was not that he couldn't detect
it, it's that there's nothing he can do about it short of extending the Logger
class *again* to make it do what he wants.
Unit tests detect the problem, they don't solve it. Since the OP controls
neither the logger package nor the glue package, his choices are limited.
Plus, I'll second Alexander's assertion that you should never have to include
unit tests to verify that classes work as advertised. That's the package
author's job, and a programmer's job not to screw with their behavior,
especially not classes in core and/or stdlib. The alternative is a horribly
slippery slope.
Regards,
Dan
James Britt
..
> Alright, rant off. Please stop doing this. My appologies if I hurt anyone's feelings.
>
I take it you first tried to contact the author of the offending code
before ranting in public?
James
--
http://www.ruby-doc.org - The Ruby Documentation Site
http://www.rubyxml.com - News, Articles, and Listings for Ruby & XML
http://www.rubystuff.com - The Ruby Store for Ruby Stuff
http://www.jamesbritt.com - Playing with Better Toys
Jeff Wood
should be asserting ALL of the behaviors you design into your
application.
What if logger isn't even being called ??? or any of a number of
situations like that... Simply verifying that the application behaves
in all ways as designed/required isn't "testing Logger".
j.
On 8/24/05, David Brady <ruby...@shinybit.com> wrote:
> Eric Hodel wrote:
>
> > I don't understand why I'm supposed to test a standard library I'm
> > using. Usually I read the documentation and expect it to work that
> > way, but maybe I'm crazy.
>
> 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.
>
> Is the following legal unit test code?
>
> def test_no_messing_with_trusted_libraries
> require 'logger'
> Logger.freeze
> assert_nothing_raised( require 'suspect_module', "Hey! Somebody's
> mucking with the logger module!" )
> end
>
> You're right, you shouldn't HAVE to run that test.
>
> -dB
>
> --
> David Brady
> ruby...@shinybit.com
> I'm feeling really surreal today... OR AM I?
Nicholas Seckar
reopened Logger. :P
class Logger
# Dictate the way in which this logger should format the messages
# it displays. This method requires a block. The block should return
# formatted strings given severity, timestamp, msg, progname.
#
# Useless example:
#
# logger = Logger.new
# logger.format do |severity, timestamp, msg, progname|
# "#{progname}@#{timestamp} - #{severity}::#{msg}"
# end
#
def format(&format_proc)
raise 'feed me a block dude' unless format_proc
@format_proc = format_proc
end
# hackish use of *args, give me some love
def format_message(*args)
@format_proc ? @format_proc.call(*args) : super(*args)
end
end
Julian Leviston
untrustworthy code and bash it into trustworthiness :)
Julian.
Austin Ziegler
>> 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.
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
--
Austin Ziegler * halos...@gmail.com
* Alternate: aus...@halostatue.ca
Jeff Wood
To answer "how/why would you test this"... you should have tests in
place for your logging functionality. The second something doesn't
come out right, you know that something is marring the system.
Debugging starts by walking the source of the packages you are using
and watching for references to the modified functionality... It's not
that hard to track down.
Anyways, that's what I do ... it works for me. As far as posting
example code ... sorry, some companies don't let ya do that.
j.
On 8/24/05, Bill Kelly <bi...@cts.com> wrote:
> From: "Jeff Wood" <jeff.da...@gmail.com>
> >
> > You shouldn't be afraid of having power. That's why you have tests.
> > You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
>
> Dudes, I do have tests. How in blazes would you write a test
> to catch the problem I described? (Please show your work. :)
> And why would you ever think to do so?
>
> It's not being afraid of having power. In my own applications,
> I'll exploit any and every cool capability of Ruby I feel like.
>
> But when I personally write a module I view as a library I'd
> like others to find useful, I take a decidedly more conservative
> approach - deliberately.
>
> Do you not consider libraries and applications different, where
> Ruby's beloved Openness is concerned?
>
>
> Regards,
>
> Bill
>
>
>
>
Jeff Wood
http://raibledesigns.com/page/rd?entry=oscon_monday_morning
---QUOTE---
Classes are open: in Ruby, you can always add methods, attributes,
etc. to existing classes. For example, you can add an encrypt() method
to the String class. Isn't this dangerous? What if someone changes the
logic of + for math expressions. No, because if one of your
programmers overrides methods that breaks things - you take them out
in the parking lot and beat them with a rubber hose! The language
shouldn't prohibit us from doing powerful things.
---END QUOTE---
You shouldn't be afraid of having power. That's why you have tests.
You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
j.
On 8/24/05, Bill Kelly <bi...@cts.com> wrote:
> From: "Zed A. Shaw" <zed...@zedshaw.com>
> >
> > I then trolled through a few more projects and found that this
> > is common practice. The glue project does it. Glue even goes
> > so far as to re-open the Logger class *just so it can turn it
> > into a singleton*. Great Buddha the insanity is everywhere!
>
> I was bitten recently by a change to the CGI module. I don't
> think I can get to quite the same level of moral outrage on this
> one because by comparison what I ran afoul of seems more like what
> one would expect to be a legitimate leveraging of the Power of Ruby,
> even in a library context: no global classes or constants were
> being opened or modified. Something that I used to convert to a
> String instance, was now still a String, but its singleton class had
> been extended with additional features. Including some accompanying
> instance variables, which is the part that bit me.
>
> My code was littered with cgi['some_parameter'].to_s
> because I'd long ago learned that CGI returned something as
> a parameter value that acted like a String, but wasn't really.
> I added the .to_s when fetching CGI parameters, because the values
> I fetch eventually end up in objects which are serialized out to
> a flat-file database with YAML.
>
> A recent change to CGI that was--probably--for most intents and
> purposes at least as good or better than the way it used to work
> was apparently to make cgi['some_parameter'] now return a genuine
> String - but one that had been extended with extra features.
> Unfortunately, this had the effect of turning my .to_s calls into
> no-op's. This had the ultimately semi-harmless but alarming and
> baffling result of my database files beginning to show signs of
> very strange bloat: objects that used to appear as simple strings
> in the YAML, were now complex dumps of objects that included the
> string value, but also these ancillary CGI parameters that were
> hitchhiking on these extended String objects CGI was now returning
> as parameter values.
>
> I think it took me at least an hour and a half to track down the
> cause. (Seemed like an eternity because I was working under a
> deadline.)
>
> To fix it, I did decide to open the String class with, well maybe
> a small set of electric hedge trimmers. But I'm only dealing with
> my private application here. If it were a library I were writing,
> I'd go to prodigious lengths to try to not modify the global behavior
> of Ruby. My cheesy application-level fix:
>
> class String
> # %%BWK -- this is a kludge to work around the CGI module extending
> # String objects fetched from its parameters with some
> # extra instance variables, which cause really verbose YAML
> # to be output.
> # I already was doing to_s on everything I fetched from
> # the CGI parameters, thinking that was enough to give me
> # a String without any extra baggage. This kludge forces
> # my assumption to be a true one.
> def to_s
> String.new(self)
> end
> end
>
> I'm not entirely sure what point I'm driving toward - because
> really what CGI is doing seems to me in general like a pretty
> reasonable use of Ruby. . . And yet it _bit_ me in a similar way to
> what you reported about the Logger Maneuver.
>
> I guess in general, Library Authors Take Note: Getting tricky in
> your library can end up playing a trick on your users.
Austin Ziegler
On 8/24/05, Jim Weirich <j...@weirichhouse.org> wrote:
> Austin Ziegler said:
>> On 8/24/05, Jeff Wood <jeff.da...@gmail.com> wrote:
>>> You shouldn't be afraid of having power. That's why you have tests.
>>> You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
>> 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".
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::Diff) 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.
David Brady
> I don't understand why I'm supposed to test a standard library I'm
> using. Usually I read the documentation and expect it to work that
> way, but maybe I'm crazy.
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.
Is the following legal unit test code?
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!
Molitor, Stephen L
numeric classes, but you explicit required those features.
I'm not sure if framework vs. library distinction holds up. Most of
Rails is structured as libraries that are usable outside of Rails
anyway. (Not sure about the date / fixnum stuff, but I would assume
so.) I think it would be ok, however, if the standard requires for
Rails (or whatever) included the extensions, but there were other
require files available that didn't:
require 'foo'
# or:
require 'foo_without_extensions'
Another question is what if you wanted to extend standard classes for
use in the internal implementation of your library. As it stands now,
that's probably not a good idea. But with the proposed Ruby 2 namespace
feature it could be safe -- it the extension would only apply within
your library.
So how about this:
1. Don't change the behavior of existing stuff.
2. Adding new methods is OK, but document clearly and make those
extensions optional.
Steve
-----Original Message-----
From: Austin Ziegler [mailto:halos...@gmail.com]
Sent: Wednesday, August 24, 2005 6:40 PM
To: ruby-talk ML
Subject: Re: Extending Code Cleanly
On 8/24/05, David Brady <ruby...@shinybit.com> wrote:
> Austin Ziegler wrote:
> >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!
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.
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.
Jim Weirich
>> You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
>
> 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".
--
-- Jim Weirich j...@weirichhouse.org http://onestepback.org
-----------------------------------------------------------------
"Beware of bugs in the above code; I have only proved it correct,
not tried it." -- Donald Knuth (in a memo to Peter van Emde Boas)
Bill Kelly
> is common practice. The glue project does it. Glue even goes
> so far as to re-open the Logger class *just so it can turn it
> into a singleton*. Great Buddha the insanity is everywhere!
I was bitten recently by a change to the CGI module. I don't
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.
Jeff Wood
a given piece of functionality is going to cause a log message to
occur, that I validate the residual log files do contain the log
entries I expected as I expected them to be formatted. If they don't
then the application/library doesn't meet it's spec and/or
requirements. It's a bug and it needs to be resolved.
So, you would then have a front-row seat if something was affecting
the logger output.
And no, I don't believe there is any difference between building a
library and building an application ... it's all code that needs to be
tested and made sure it functions completely as designed.
I hope that answers you sufficiently.
j.
On 8/24/05, Bill Kelly <bi...@cts.com> wrote:
Eric Hodel
> Guys,
>
> I don't understand. This is a feature of agile languages, not a
> defect. Leaving the door open for chainsaw-wielding maniacs also
> leaves the door open for gifted neurosurgeons.
>
> I understand your frustration, and I feel your pain. But I am left
> with a critical question:
>
> Where were your unit tests?
I don't understand why I'm supposed to test a standard library I'm
using. Usually I read the documentation and expect it to work that
way, but maybe I'm crazy.
> Admittedly, silence_warnings might not be testable. You might have
> to code review that to catch it. But then again, if *your* unit
> tests all pass, then silence_warnings shouldn't affect you--
> especially if you test the failure cases and assert that warnings
> were returned.
Right now, silence_warnings is only used in the library's test code,
but that doesn't mean it won't get abused and start wrapping things
that *should* give warnings. (Of course, the library in question is
so completely -w unsafe, the warnings would be lost in a pages of
noise.)
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!).
> Logger integrity, however, should be straightforward.
You would think that, until one library decides to go ahead and break
it. The library where these were found has at least a handful of
places where you can find questionable software engineering practices.
> I don't mean to come across like a heartless jerk or a TDD zealot.
> I am neither. I am just looking at your pain and thinking, "why
> does this have to hurt?"
It hurts because the library decided to us in the middle of a
minefield without a map.
--
Eric Hodel - drb...@segment7.net - http://segment7.net
FEC2 57F1 D465 EB15 5D6E 7C11 332A 551C 796C 9F04
Nikolai Weibull
> ---QUOTE---
>
> Classes are open: in Ruby, you can always add methods, attributes,
> etc. to existing classes. For example, you can add an encrypt() method
> to the String class. Isn't this dangerous? What if someone changes the
> logic of + for math expressions. No, because if one of your
> programmers overrides methods that breaks things - you take them out
> in the parking lot and beat them with a rubber hose! The language
> shouldn't prohibit us from doing powerful things.
>
> ---END QUOTE---
Violence is surely the road to enlightenment,
nikolai
--
Nikolai Weibull: now available free of charge at http://bitwi.se/!
Born in Chicago, IL USA; currently residing in Gothenburg, Sweden.
main(){printf(&linux["\021%six\012\0"],(linux)["have"]+"fun"-97);}
Jeff Wood
design and implement your tests before you write your code. When all
of your tests pass, you're done...
.. If you built your system like that, you should have tests for each
and every action on your classes ... and the first time somebody elses
modifications has an affect on your functionality and expected output,
you should know about it.
j.
On 8/24/05, Bill Kelly <bi...@cts.com> wrote:
> From: "Jeff Wood" <jeff.da...@gmail.com>
> >
> > You shouldn't be afraid of having power. That's why you have tests.
> > You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
>
> Dudes, I do have tests. How in blazes would you write a test
> to catch the problem I described? (Please show your work. :)
> And why would you ever think to do so?
>
> It's not being afraid of having power. In my own applications,
> I'll exploit any and every cool capability of Ruby I feel like.
>
> But when I personally write a module I view as a library I'd
> like others to find useful, I take a decidedly more conservative
> approach - deliberately.
>
> Do you not consider libraries and applications different, where
> Ruby's beloved Openness is concerned?
>
>
David Brady
I don't understand. This is a feature of agile languages, not a
defect. Leaving the door open for chainsaw-wielding maniacs also leaves
the door open for gifted neurosurgeons.
I understand your frustration, and I feel your pain. But I am left with
a critical question:
Where were your unit tests?
Admittedly, silence_warnings might not be testable. You might have to
code review that to catch it. But then again, if *your* unit tests all
pass, then silence_warnings shouldn't affect you--especially if you test
the failure cases and assert that warnings were returned. Logger
integrity, however, should be straightforward.
I don't mean to come across like a heartless jerk or a TDD zealot. I am
neither. I am just looking at your pain and thinking, "why does this
have to hurt?"
-dB
Austin Ziegler
> >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!
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
Alexandru Popescu
> [NOTE: This is an offshoot of Zed Shaw's rant on Chainsaw Infanticide.]
>
>> "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::Diff) 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::.|
David Brady
>Again, I'd ask to see what sort of test you'd propose that would
>catch the problem I described. (My post, the one you quoted, was
>about CGI, not the Logger.)
>
>
I find that the hardest tests to write are the ones that I overlooked
because I am unaware of my blind spots.
Until I read your post, I thought like you did--that to_s flattened an
object into a chunk of text. But now I see that it doesn't. It doesn't
even return a String object! It returns an object that behaves--dare I
say quacks?--like a String.
Another assumption you may be making--and once I state it, the test for
it should become obvious--is that YAML should output pleasingly human
readable text, even if it loses the ability to correctly restore objects.
I know, I know. There exists in business the concept of "approprately
incorrect". You are using YAML to produce "appropriately incorrect" files.
The behavior you want is "take a Stringlike object, and emit pleasingly
simplified text". That's easily testable. Take your CGI object, have
YAML dump it, then open the YAML file manually and see that the text is
correct. Or build a CGI object, then build an equivalent object using
Strings, output them to different files and then assert that there is no
difference between the files. The second method may seem more robust,
because it lets you not care how YAML stores Strings... but that's the
rabbit that led us down this hole in the first place. Use the first
method, because you don't care if YAML stores CGI the same as a String,
you care that the output is pleasing to read. If YAML changes the way
it stores String, you'll want this test to break. Your app should
shriek until you have verified that the new format is pleasant.
Once you have a regression test in place, you will be able to remove the
hack to String#to_s and know when your application is fixed. A good
alternative to your hack, perhaps, is to replace YAML with a dumper of
your own devising that transforms Stringlike-->String on save/reload.
Perhaps even a child class of YAML, whose only override is something like:
obj = String.new(obj) if obj.respond_to? :to_str && obj.class != String
I haven't learned how to see my blind spots yet. I could not have
prevented this bug from happening. But having happened, I can
immediately write a test that verifies that it is fixed, and that future
revisions to CGI, YAML, and even String will be regression tested
against this bug.
Cheers,
Bill Kelly
>
> You shouldn't be afraid of having power. That's why you have tests.
> You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
Dudes, I do have tests. How in blazes would you write a test
Eric Hodel
> 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.
>
> Three hours later and half my sanity, I start to snoop through the
> libraries I've included, and I find this wonderfully horrible gem:
>
> http://rafb.net/paste/results/0vADxJ56.html
Oh? You think that's bad?
There's also this gem sitting right next to it.
def silence_warnings
old_verbose, $VERBOSE = $VERBOSE, nil
begin
yield
ensure
$VERBOSE = old_verbose
end
end
So that things like this can be done:
silence_warnings { Customer = Struct.new("Customer", :name) }
instead of doing something simple like:
unless defined? Customer
Customer = Struct.new "Customer", :name
end
Austin Ziegler
> You do have tests, right? ... right??? ... RIGHT !!?!?!?!??!
I think the real problem is when this is done in released code. If
you're going to extend code, extend it cleanly -- IMO.
-austin
Bill Kelly
Thanks for your thoughtful reply.
From: "David Brady" <ruby...@shinybit.com>
>
> Bill Kelly wrote:
>
> >Again, I'd ask to see what sort of test you'd propose that would
> >catch the problem I described. (My post, the one you quoted, was
> >about CGI, not the Logger.)
> >
> >
> I find that the hardest tests to write are the ones that I overlooked
> because I am unaware of my blind spots.
Yes - agreed.
> Until I read your post, I thought like you did--that to_s flattened an
> object into a chunk of text. But now I see that it doesn't. It doesn't
> even return a String object! It returns an object that behaves--dare I
> say quacks?--like a String.
>
> Another assumption you may be making--and once I state it, the test for
> it should become obvious--is that YAML should output pleasingly human
> readable text, even if it loses the ability to correctly restore objects.
Hmm... Actually my assumption (or hope) would be that YAML always
be able to write out objects that it can correctly restore. My
reason for wanting my application to be serializing "real" simple
strings isn't really tied, I think, to any assumptions about how
YAML itself should behave.
But yes, I want my *application* to, using YAML, output simple
strings that are not only pleasingly human readable, but also
compact*. I have no problem with YAML doing exactly what I've
asked it to. I just didn't realize I was feeding it complex
singleton String instances with "extra" instance variables.
(*) The compactness is desirable for performance reasons:
I'm already splitting my databases via hashing into 4093 sub-
files, so each component file of the database can be loaded/saved
quickly enough. YAML Ain't Mercurial Lightning... ? ;)
> I know, I know. There exists in business the concept of "approprately
> incorrect". You are using YAML to produce "appropriately incorrect" files.
I'm not sure I agree. I'm using YAML to accurately serialize my
data. It's my fault for feeding it unexpectedly complex data.
> The behavior you want is "take a Stringlike object, and emit pleasingly
> simplified text". That's easily testable. Take your CGI object, have
> YAML dump it, then open the YAML file manually and see that the text is
> correct. Or build a CGI object, then build an equivalent object using
> Strings, output them to different files and then assert that there is no
> difference between the files. The second method may seem more robust,
> because it lets you not care how YAML stores Strings... but that's the
> rabbit that led us down this hole in the first place. Use the first
> method, because you don't care if YAML stores CGI the same as a String,
> you care that the output is pleasing to read. If YAML changes the way
> it stores String, you'll want this test to break. Your app should
> shriek until you have verified that the new format is pleasant.
One test I had in mind was,
assert( cgi['something'].to_s.to_yaml ==
String.new(cgi['something'].to_s).to_yaml )
..but that test does rely on my modified String#to_s behavior.
One complication is that, for the time being, my application needs
to run on systems with the 1.8.1 CGI behavior (which returns a
parameter value as a non-String-object that quacks like a String),
and the 1.8.2 CGI behavior (which returns a paramater value as a
genuine String instance whose singleton class happens to have been
extend'ed with a module that includes some bonus instance variables.)
So far, my String#to_s hack still seems like a reasonable enough
way to enforce the behavior I need in my current application. (But
again, I'd NOT take this hack approach if I were writing a library.)
So, given my String#to_s hack, I think the above assert() should
cover the specific problem I was having.
A more direct approach might have been for me to open the CGI class
and replace CGI#[] so that it returned simple String instances, not
complex extend'ed ones. (Again, something I'd consider for my app,
but not for a library.)
I'm not sure if more general tests involving YAML and Strings would
help, since I don't *really* want to change YAML's behavior - I just
want to not accidentally feed it complex objects.
Regards,
Bill
Gavin Kistner
[... snip an awesome tirade...]
ROFLMAO. Well written :)
"Principle Of Most Heinous Arsenic Injection"...*giggle*
Thanks for the laugh. I agree with your sentiments, and the way
you've written them.
gabriele renzi
> 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
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 :pre, :post, :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
=> '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
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
> On 8/24/05, Zed A. Shaw <zed...@zedshaw.com> wrote:
>
>>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?
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
;)
nll...@gmail.com
> 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.
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