Forum: Ruby Methods validating their arguments: good or bad?

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Michael J. (Guest)
on 2006-12-30 00:36
(Received via mailing list)
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 J.

* Code Complete is a book on software construction by Steve McConnell.
Nick P. (Guest)
on 2006-12-30 01:20
(Received via mailing list)
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 J. wrote:

> it?  Is it too much overhead for too little gain?
> Plus it would probably need a few helper methods just to remain
>
>
--

Nick P.
removed_email_address@domain.invalid
www.skatingloons.com
Vincent F. (Guest)
on 2006-12-30 01:36
(Received via mailing list)
Michael J. 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
Gavin K. (Guest)
on 2006-12-30 01:50
(Received via mailing list)
Vincent F. 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 K. (Guest)
on 2006-12-30 01:56
(Received via mailing list)
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 F. (Guest)
on 2006-12-30 02:12
(Received via mailing list)
Phrogz wrote:
> -------------------------------------------- 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
Robert D. (Guest)
on 2006-12-30 10:35
(Received via mailing list)
On 12/29/06, Michael J. <removed_email_address@domain.invalid> wrote:
I *was* quite radically for early checking and got lot`s of arguing, I
piped
down than and
started thinking why something *so* obvious was not wildely recognized
by
the community.
I had some ideas about it may I?

Code Complete* recommends that methods validate the arguments they
> receive.  The author went on to say that in a 1984 study,


That ´s it 1984 I was still in university, there was C, Pascal, Algol,
Ada83, C++(just I guess), Lisp Fortran Cobol - there was Smalltalk too,
but
who considered it in those times .
There was no TDD or BDD, there was no Extreme Programming, Testing was
just
not part of development.
How much do such studies apply to modern agile languages which *should*
use
a different developement approach?

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.


It might be much tougher to do that than to say that.

  Should
> 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.


This is a very good case of yours as we are talking about an exposed
API,
and thus TDD cannot really apply as in a closed (project) API where
tests
should catch a many errors.
Still I think it is not the early checking which is missing most but
maybe
documentation?

The argument for adding validation is really strong for any non-
> trivial project, but does it ruin Ruby's elegance?


Yes I am afraid so, and furthermore the validation argument is something
I
feel I carry as a burden from my Ada and Pascal past, and it is not as
important as I felt.

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


Less and less, but the less I validate the more I develop in TDD (or BDD
if
I am in a good mood).
OTH I would still tend to validate a lot if I were exposing an API, that
seems the crucial point to discuss to me.

- Michael J.
>
> * Code Complete is a book on software construction by Steve McConnell.
>
>
Cheers
Robert

--
"The real romance is out ahead and yet to come. The computer revolution
hasn't started yet. Don't be misled by the enormous flow of money into
bad
defacto standards for unsophisticated buyers using poor adaptations of
incomplete ideas."

- Alan Kay
Rob S. (Guest)
on 2006-12-30 11:07
(Received via mailing list)
On 12/29/06, Michael J. <removed_email_address@domain.invalid> wrote:
> we do it?  Is it too much overhead for too little gain?
> Plus it would probably need a few helper methods just to remain
>
>

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 (Guest)
on 2006-12-30 12:01
(Received via mailing list)
Vincent F. wrote:
> begin
> 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 Y. (Guest)
on 2006-12-30 14:15
(Received via mailing list)
Michael J. wrote:
> it?  Is it too much overhead for too little gain?
> 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.html#6.2.0

I do it (not always this way), but not often enough...
Paulo Köch (Guest)
on 2006-12-30 18:58
(Received via mailing list)
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 J. (Guest)
on 2006-12-31 00:03
(Received via mailing list)
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 J.
unknown (Guest)
on 2006-12-31 00:51
(Received via mailing list)
Hi --

On Sat, 30 Dec 2006, Michael J. 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.

The checking doesn't happen until the method is called, though.

> 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.

What kind and amount of validation are you thinking of?

> 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?

It depends a bit on what you mean by validate.  It's definitely good
for methods to handle bad data gracefully, but sometimes just letting
Ruby fail and raise an exception is more graceful than trying to
figure out in a hands-on way what's going to go wrong and then raising
the exception yourself.


David
Michael J. (Guest)
on 2006-12-31 10:09
(Received via mailing list)
Hi,

On Dec 30, 2006, at 2:50 PM, removed_email_address@domain.invalid wrote:

>> 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.
>
> The checking doesn't happen until the method is called, though.

You're right.  That's an important distinction.

> Ruby fail and raise an exception is more graceful than trying to
> figure out in a hands-on way what's going to go wrong and then raising
> the exception yourself.
>
> David

I very much agree with you.  I love when Ruby fails.  The stack trace
and error message are all I need to get back on track after a snafu.
But what about when Ruby doesn't fail?  I'm not talking about syntax
problems, but just general programmer stupidity, like when I
blatantly assign the wrong object to a temporary variable and then
send it off somewhere.  That's the part I want to trap.  Actually, I
want to trap it, strip it naked, and parade it up and down the street.

#
# Example of Argument Validation
#

require 'validate_arguments'

def print_report(record)

   validate do
     hash_role record, :keys => [:id, :status]
     numeric_role record[:id], :values => [0..100]
     symbol_role record[:status], :values => [:failed, :passed]
   end

   puts <<-END
     Report #{record[:id]}
     Status: #{record[:status]}
     -----
   END
end

# The :id key should be a number, but that would make for a crappy
example.
print_report :id => [1,2], :status => :passed

# Let's try sending it a different key than :id too.
print_report :number => 2, :status => :failed

#
# Finish
#

Without argument validation, it would print this.  (Egads!)

     Report 12
     Status: passed
     ------

     Report
     Status: failed
     ------

But with the validate block in place, it fails:

ArgumentError: Object should be a number-like object
         from (irb):7:in `print_report'
         from ./validate_arguments.rb:3:in `validate'
         from (irb):5:in `print_report'
         from (irb):18
         from :0

ArgumentError: Hash-like object is missing key :id
         from (irb):6:in `print_report'
         from ./validate_arguments.rb:3:in `validate'
         from (irb):5:in `print_report'
         from (irb):19
         from :0

That's better than a broken report, right?

There's probably a better solution to this (like, drinking less
tequila.)  But I feel pretty good about putting little
straightjackets on my variables.  I think they feel loved.

Kind regards,

Michael J.
Kenosis (Guest)
on 2007-01-03 01:00
(Received via mailing list)
Michael J. wrote:
> >> receive. The author went on to say that in a 1984 study,
> You're right.  That's an important distinction.
> > for methods to handle bad data gracefully, but sometimes just letting
> blatantly assign the wrong object to a temporary variable and then
>
>    END
> # Finish
>      ------
> ArgumentError: Hash-like object is missing key :id
> straightjackets on my variables.  I think they feel loved.
>
> Kind regards,
>
> Michael J.

I think the first reply to your post hits the nail on the head: in the
book Object Oriented Software Construction and its presentation of
Design by Contract, the level of pre, post, and invariant condition
checking really tends to be a matter of the reliability requirements
your working with.  And with Ruby there are many creative ways to
accomplish this (DbC) but by and large they are still all "on your
honor" techniques.  The author (Meyer) also argues that one needs
support intrinsic to the programming language to really get the full
benefit of DbC but in my personal experience a LOT of benefit can be
had even by on your honor techniques, like the some of the ones
mentioned above.

Ken
This topic is locked and can not be replied to.