Asp Forum - Methods validating their arguments: good or bad?

Michael Judge

12/29/2006 10:34:00 PM

Code Complete* recommends that methods validate the arguments they
receive. The author went on to say that in a 1984 study, researchers
discovered that miscommunication between routines caused 39% of the
programming errors observed. The idea is that, if we check our
arguments before using them, bad data won't flow through undetected,
causing even vaguer problems later on. Lucky for us, Ruby checks the
number of arguments received against the method definition -- finding
many of these problems at the time they're written. But checking
that the arguments make sense are still our responsibility. Should
we do it? Is it too much overhead for too little gain?

From the Rails source:

def select_tag(name, option_tags = nil, options = {})
content_tag :select, option_tags, { "name" => name, "id" =>
name }.update(options.stringify_keys)
end

The arguments to select tag are really easy to screw up. Adding code
to validate them would add four or five lines to this terse method.
Plus it would probably need a few helper methods just to remain
readable. But it would also prevent programmers from wasting huge
amounts of time hunting for why their browser rendered a bad select
box. The argument for adding validation is really strong for any non-
trivial project, but does it ruin Ruby's elegance?

What do you think? Do you usually validate your arguments?

- Michael Judge

* Code Complete is a book on software construction by Steve McConnell.

13 Answers

Nick Pavey

12/29/2006 11:19:00 PM

Hi,

I don't check my pre and post conditions all the time, although I keep
meaning to do more of it.

However, one of the times that I did add the functionality (in Perl) it
definitely saved be from some gnarly, hard to find bugs.

So, like most things I guess, it depends on the situation. If
reliability is your goal then, yes, it can really help. A compromise
I've used is to add tougher error checking on the places where bad
arguments could end up injecting a booby trap for later on.

Cheers,

Nick

On Dec 29, 2006, at 5:34 PM, Michael Judge wrote:

>
> Code Complete* recommends that methods validate the arguments they
> receive. The author went on to say that in a 1984 study, researchers
> discovered that miscommunication between routines caused 39% of the
> programming errors observed. The idea is that, if we check our
> arguments before using them, bad data won't flow through undetected,
> causing even vaguer problems later on. Lucky for us, Ruby checks the
> number of arguments received against the method definition -- finding
> many of these problems at the time they're written. But checking that
> the arguments make sense are still our responsibility. Should we do
> it? Is it too much overhead for too little gain?
>
> From the Rails source:
>
> def select_tag(name, option_tags = nil, options = {})
> content_tag :select, option_tags, { "name" => name, "id" => name
> }.update(options.stringify_keys)
> end
>
> The arguments to select tag are really easy to screw up. Adding code
> to validate them would add four or five lines to this terse method.
> Plus it would probably need a few helper methods just to remain
> readable. But it would also prevent programmers from wasting huge
> amounts of time hunting for why their browser rendered a bad select
> box. The argument for adding validation is really strong for any
> non-trivial project, but does it ruin Ruby's elegance?
>
> What do you think? Do you usually validate your arguments?
>
> - Michael Judge
>
> * Code Complete is a book on software construction by Steve McConnell.
>
>
--

Nick Pavey
nick@skatingloons.com
www.skatingloons.com

Vincent Fourmond

12/29/2006 11:35:00 PM

Michael Judge wrote:
> What do you think? Do you usually validate your arguments?

I usually don't, first because I find it makes the code less readable,
and second because of the performance loss. What I would like, however,
is something Eifel-like:

def method(a)
entry_check
a.is_a(String)
begin
# now do something with code
exit_check
# a test on exit
end

And the possibility to simply turn it off, that is, completely skip
over the entry_check and exit_check blocks.

What do you think ? I think that it would be a marvellous addition to
Ruby. Of course, that doesn't stop you from writing Test::Units, that
would be somehow orthogonal.

Cheers,

Vince

--
Vincent Fourmond, PhD student
http://vincent.fourmon...

Gavin Kistner

12/29/2006 11:46:00 PM

Vincent Fourmond wrote:
> def method(a)
> entry_check
> a.is_a(String)
> begin
> # now do something with code
> exit_check
> # a test on exit
> end

An idea:

if $DEBUG
def entry_check; yield; end
else
def entry_check; end
end
alias_method :exit_check, :entry_check

def method( a )
entry_check{
raise "OUCH" unless a.is_a?( String )
}
# meat here...
exit_check{
# ...
}
end

When the debug flag is supplied, the block is yielded and run. When
not, the entry_check call still does have the overhead of a do-nothing
method call, but far less than before.

Gavin Kistner

12/29/2006 11:55:00 PM

Phrogz wrote:
> When the debug flag is supplied, the block is yielded and run. When
> not, the entry_check call still does have the overhead of a do-nothing
> method call, but far less than before.

Some followup performance tests from this idea:

def check; yield; end
def nothing; end

def no_test( str )
str[ 1..3 ]
end

def test_inline( str )
raise "OUCH" unless str.is_a?( String ) && str.length > 3
str[ 1..3 ]
end

def test_through_block( str )
check{
raise "OUCH" unless str.is_a?( String ) && str.length > 3
}
str[ 1..3 ]
end

def dead_method_call( str )
nothing{
raise "OUCH" unless str.is_a?( String ) && str.length > 3
}
str[ 1..3 ]
end

require 'benchmark'

s = "Hello"
N = 1_000_000
Benchmark.bmbm{ |x|
x.report( 'no_test' ){ N.times{ no_test( s ) } }
x.report( 'dead_method_call' ){ N.times{ dead_method_call( s ) } }
x.report( 'test_inline' ){ N.times{ test_inline( s ) } }
x.report( 'test_through_block' ){ N.times{ test_through_block( s ) }
}
}
__END__
Rehearsal ------------------------------------------------------
no_test 2.930000 0.020000 2.950000 ( 2.999768)
dead_method_call 4.030000 0.050000 4.080000 ( 4.232545)
test_inline 4.830000 0.050000 4.880000 ( 5.065821)
test_through_block 6.100000 0.050000 6.150000 ( 6.227374)
-------------------------------------------- total: 18.060000sec

user system total real
no_test 2.920000 0.020000 2.940000 ( 2.971835)
dead_method_call 3.910000 0.030000 3.940000 ( 3.990496)
test_inline 4.750000 0.040000 4.790000 ( 4.825439)
test_through_block 6.140000 0.050000 6.190000 ( 6.263266)

Vincent Fourmond

12/30/2006 12:10:00 AM

Phrogz wrote:
> Phrogz wrote:
>> When the debug flag is supplied, the block is yielded and run. When
>> not, the entry_check call still does have the overhead of a do-nothing
>> method call, but far less than before.
>
> Rehearsal ------------------------------------------------------
> no_test 2.930000 0.020000 2.950000 ( 2.999768)
> dead_method_call 4.030000 0.050000 4.080000 ( 4.232545)
> test_inline 4.830000 0.050000 4.880000 ( 5.065821)
> test_through_block 6.100000 0.050000 6.150000 ( 6.227374)
> -------------------------------------------- total: 18.060000sec
>
> user system total real
> no_test 2.920000 0.020000 2.940000 ( 2.971835)
> dead_method_call 3.910000 0.030000 3.940000 ( 3.990496)
> test_inline 4.750000 0.040000 4.790000 ( 4.825439)
> test_through_block 6.140000 0.050000 6.190000 ( 6.263266)

Well, that is something, but that really calls for a dedicated
implementation, unless the performance loss isn't that great with Yarv.
A dedicated implementation could run with hardly any overhead, and that
would be fantastic. Actually, if that interest some people, I'd like to
give it a try.

Vince

--
Vincent Fourmond, PhD student
http://vincent.fourmon...

Rob Sanheim

12/30/2006 9:07:00 AM

On 12/29/06, Michael Judge <mjudge@surveycomplete.com> wrote:
>
> Code Complete* recommends that methods validate the arguments they
> receive. The author went on to say that in a 1984 study, researchers
> discovered that miscommunication between routines caused 39% of the
> programming errors observed. The idea is that, if we check our
> arguments before using them, bad data won't flow through undetected,
> causing even vaguer problems later on. Lucky for us, Ruby checks the
> number of arguments received against the method definition -- finding
> many of these problems at the time they're written. But checking
> that the arguments make sense are still our responsibility. Should
> we do it? Is it too much overhead for too little gain?
>
> From the Rails source:
>
> def select_tag(name, option_tags = nil, options = {})
> content_tag :select, option_tags, { "name" => name, "id" =>
> name }.update(options.stringify_keys)
> end
>
> The arguments to select tag are really easy to screw up. Adding code
> to validate them would add four or five lines to this terse method.
> Plus it would probably need a few helper methods just to remain
> readable. But it would also prevent programmers from wasting huge
> amounts of time hunting for why their browser rendered a bad select
> box. The argument for adding validation is really strong for any non-
> trivial project, but does it ruin Ruby's elegance?
>
> What do you think? Do you usually validate your arguments?
>
> - Michael Judge
>
> * Code Complete is a book on software construction by Steve McConnell.
>
>

It depends.

Public APIs used by many clients, or even by the world via a service?
I would say at least consider validation. There are a bunch of good
examples of how to do it declaratively and cleanly in the Ruby
Cookbook, and of course in the archives.

Internal apis, protected and private methods? Eh, not so much.

Unless that internal api has some crazy parameter list it expects that
we'll be tough to figure out via just the code and tests. In that
case, it again may be worth it. Of course, that kind of thing is a
code smell in of itself...but sometimes its easier to raise an
exception with a decent error message then refactor crufty code.

- Rob

AdSR

12/30/2006 9:58:00 AM

Vincent Fourmond wrote:
> Michael Judge wrote:
> > What do you think? Do you usually validate your arguments?
>
> I usually don't, first because I find it makes the code less readable,
> and second because of the performance loss. What I would like, however,
> is something Eifel-like:
>
> def method(a)
> entry_check
> a.is_a(String)
> begin
> # now do something with code
> exit_check
> # a test on exit
> end
>
> And the possibility to simply turn it off, that is, completely skip
> over the entry_check and exit_check blocks.
>
> What do you think ? I think that it would be a marvellous addition to
> Ruby. Of course, that doesn't stop you from writing Test::Units, that
> would be somehow orthogonal.
>
> Cheers,
>
> Vince

What you could do is create a subclass with checking, something like
this (untested):

class Unchecked
def method(a)
# do your stuff
end
end

class Checked < Unchecked
def method(a)
# do pre-ckecking
result = super.method(a)
# do post-checking
result
end
end

Depending on whether you're testing or running in production you'd
switch the actual class used.

If you use this pattern a lot, maybe you could create some general
decorator class (using method_missing etc.) that would allow you to
specify tests for each decorated class or object.

Just a hint. Cheers,

AdSR

Alex Young

12/30/2006 12:14:00 PM

Michael Judge wrote:
>
> Code Complete* recommends that methods validate the arguments they
> receive. The author went on to say that in a 1984 study, researchers
> discovered that miscommunication between routines caused 39% of the
> programming errors observed. The idea is that, if we check our
> arguments before using them, bad data won't flow through undetected,
> causing even vaguer problems later on. Lucky for us, Ruby checks the
> number of arguments received against the method definition -- finding
> many of these problems at the time they're written. But checking that
> the arguments make sense are still our responsibility. Should we do
> it? Is it too much overhead for too little gain?
>
> From the Rails source:
>
> def select_tag(name, option_tags = nil, options = {})
> content_tag :select, option_tags, { "name" => name, "id" => name
> }.update(options.stringify_keys)
> end
>
> The arguments to select tag are really easy to screw up. Adding code to
> validate them would add four or five lines to this terse method. Plus
> it would probably need a few helper methods just to remain readable.
> But it would also prevent programmers from wasting huge amounts of time
> hunting for why their browser rendered a bad select box. The argument
> for adding validation is really strong for any non-trivial project, but
> does it ruin Ruby's elegance?
>
> What do you think? Do you usually validate your arguments?
>

This has come up before:

http://www.erikveen.dds.nl/monitorfunctions/index....

I do it (not always this way), but not often enough...

--
Alex

Paulo Köch

12/30/2006 4:58:00 PM

On 2006/12/29, at 23:50, Phrogz wrote:

> def method( a )
> entry_check{
> raise "OUCH" unless a.is_a?( String )
> }
> # meat here...
> exit_check{
> # ...
> }
> end

It's quite possible to think that the raise is, in fact, "assert
a.is_a? String", reenforcing that this is quite the same as testing
arguments on the fly, with each method call. OTOH, isn't this
remarkably similar to static typing? Think of the java's interface
hell (having to implement enormous amounts of interfaces in one
class). We're just delegating type checking to pre-call tests,
assuring that the duck we're receiving as an argument can quack as we
would except it to. I think type checking is not such a good idea.
This kind of mechanism should assert if the behavior of the arguments
makes them plausible to be arguments. I'll try to exemplify.

Consider a method that prints the length of an object (obj.length) in
roman numerals. If the object doesn't have the length method, the
method should degrade gracefully and exit without printing nothing.

Consider a "Project" object that can only have a manager that has, at
least, 5 years experience. In this case, the method should raise a
NotEnoughExperienceException.

Putting it in a _why-ish way, the pre-call tests should not check if
your duck can quack but instead test that that quack's pitch is high
enough for the method to be applied to it.

These are just my humble opinions. Not authoritative in any way.

Note: this is beginning to look like formal methods of ensuring code
and runtime correctness. History has it that they're always resource
consuming and it's cheaper to read the documentation, sanitize inputs
and code in a properly fashion.

Michael Judge

12/30/2006 10:02:00 PM

Hey Everyone,

Ruby has a reputation for having a great community and it's very
true. I'm floored by the responses to this thread. You guys and
girls are all really bright individuals, and I'm humbled by your
goodwill. It's been really thought-provoking for me to read your ideas.

I'm totally into the idea. I program surveys for market research
companies, so I'm going to gamble that it's going to be worth trying
in the next version of my survey API.

Kind regards,
Michael Judge

comp.lang.ruby

Methods validating their arguments: good or bad?

Michael Judge

Nick Pavey

Vincent Fourmond

Gavin Kistner

Gavin Kistner

Vincent Fourmond

Rob Sanheim

AdSR

Alex Young

Paulo Köch

Michael Judge

x Login to ForumsZone