(defined($coin) && $coin == 25) || warn("You must insert 25 cents"), return 0;

Should be either

(defined($coin) && $coin == 25) or warn("You must insert 25 cents"), return 0;

or

(defined($coin) && $coin == 25) || (warn("You must insert 25 cents"), return 0);

Otherwise it will always return. I personally always use or for these kinds of idioms, as it is more likely to have the precedence I want.

Many thanks, I update the OP too.

-stvn

See also Re: A new idiom -or- I Hate Unless.

It is not enough to have perl complain the first time I try to do $arg->{"key"} on an array ref. I want to catch things prior to that, and not allow any part of my methods to be run unless it receives the proper input.

Design By Contract does not seem to solve the problem, it only makes language translator complain a little earlier, does it? We need to test the code with both correct and incorrect input values in any case.

Design By Contract does not seem to solve the problem, it only makes language translator complain a little earlier, does it?

Yes, but nothing can solve that problem really, since its "user error". I forgot where I heard it, but I have always like the quote: "Complain early and often" when speaking about error handling.

We need to test the code with both correct and incorrect input values in any case.

I agree. My point is that you can efficiently create those tests using this style and not have the overhead of full blown conditional blocks.

-stvn

Hm. Imagine a tool that takes all our declared contracts and composes a huge test suite that tries to test whether each contract is respected in our code, say, by feeding a series of valid and invalid data and turning off all runtime contract checks.

E.g. I declare that my complex compute_graviton_phasor_coefficient sub wants to take a positive prime number as argument. Now I'd like to get a test that will run it against random numbers and testing that it dies in the right places.

And, putting dreams aside, I'm going now to read about specification testing and try Test::LectroTest :)

Here's one that will make your brane hurt ...

my $arrayref = [qw(i like pie)];
bless($arrayref, 'HASH');
[download]

Dealing with this correctly is on my to-do list for Data::Compare. It will probably involve lots of eval evil.

Yeah, this is a hard one, but IMO, if you do such things, you should expect programs to die horrible deaths because of it, and the blame falls on your code not the modules parameter checking code.

This is one of the important points of Design By Contract (and all contracts for that matter), you must abide by the provisions of the contract, and if you try to subvert the contract, there should be reprocusions.

-stvn

Perhaps instead of using ref you should check out Scalar::Util's reftype?

antirice    
The first rule of Perl club is - use Perl
The ith rule of Perl club is - follow rule i - 1 for i > 1

The only real solution to this problem is to see if you can use something as the type of ref you want:

sub hashref { eval { \%{$_[0]} } }
sub myfunc {
   hashref($_[0]) or die "myfunc expected a hashref";
}
[download]

reftype won't handle all cases (e.g. deref-overloading or perl4-style glob passing).

Yes, use assertions. Use boatloads of them.

 "assertions++" x 1000. Assertions rock. You will be surprised at how many bugs they detect.

And if you can (risk, speed, etc), leave them in and turned on in production code (with an appropriate handler, of course). (After all, these are assertions, and thus should NEVER happen, right?)

but...

Unless you're writing time-critical software in perl -- a game, a controller, etc (and one could ask, "errr, why in perl, then?" if that is indeed the case) -- then

1. The program run time will very likely be dominated by IO, usually disk, db, network, web fetches, etc, and thus the speed difference between  assert (defined $foo, 'defined foo') and  defined $foo || die 'defined foo'; should be irrelevent in real code
2. Machine time is cheap and Programmer time is expensive -- what is clearest? Which is easiest for others to read?
3. Making assert a sub makes it Very Easy to change the assert behavior of the whole code base (logging, reponding to errors, correct shutdown in case of Really Bad Errors, etc), whereas the in-line || doesn't give you those options.

And as for ensuring an entire code base meets explicit coding standards -- well, this is great use of perl testing. just File::Find::Rule or whatever and  Test::More the whole source tree and apply regexps to every module and program, such that code that doesn't meet standard fails the test and 'breaks the build'.

From what I've read here, stvn is a far more advanced coder than I. This is not a criticism of his post or his method, just a caution to the less-experienced: Premature optimization is the root of all evil .

Don't worry about speed difference until you know your program is running too slowly, then rationally determine what is creating the bottleneck. And I'd bet you lunch that, unless you have contrived code, the bottleneck won't be your assertions. No way.

 
"assertions++" x 1000;
"coding_standards++" x 1000;
"'worrying about speed of assert vs. ||' -- ";
[download]

water,

Excellent points, all of them, however I do disagree with a few of them.

And if you can (risk, speed, etc), leave them in and turned on in production code (with an appropriate handler, of course). (After all, these are assertions, and thus should NEVER happen, right?)

This I do not disagree with, but instead strongly agree with. After all, if one of your pre-conditons is that you get passed a connected database handle, or a writeable filehandle, would you not want to check that at runtime as well? Assertions and Contracts are not just for debugging, they are ways in which you can make your code more reliable and robust.

The program run time will very likely be dominated by IO, usually disk, db, network, web fetches, etc, and thus the speed difference between  assert (defined $foo, 'defined foo') and  defined $foo || die 'defined foo'; should be irrelevent in real code

This is true, assertions will almost never be your bottleneck, but personally I have assertions just about everywhere, which includes methods and functions which need to get called inside tight loops, as well as at time critical portions of code.

For instance I have a part of a reporting app which spends a lot of time doing a DB query, once I get the results back (up to 30,000+ rows) I need to loop through them all and calculate various values. My query has already taken a long time, so anything more I do, I want it to be fast so that the user doesn't think somethings wrong. I use these fast assertions inside the calcualtion routines and loops to make sure my values are always valid before I start. Doing this has the added benefit of keeping my calculation code simple and fast since it never gets an edge case so it doesnt need to handle them.

I guess my point is that, yes, IO/Network/DB stuff can take a while and certainly be the likelest bottleneck, but sometimes this means your post-processing code needs to be that much faster.

Machine time is cheap and Programmer time is expensive -- what is clearest? Which is easiest for others to read?

Agreed, but IMHO, my way is clearer :)

You say TOmato, I say TomAto. It's all a matter of style.

Making assert a sub makes it Very Easy to change the assert behavior of the whole code base (logging, reponding to errors, correct shutdown in case of Really Bad Errors, etc), whereas the in-line || doesn't give you those options

Not so, a modification of the example above will show that it can be done:

sub insertQuarter {
    my ($coin) = @_;
    (defined($coin) && $coin == 25) 
        || errorHandler(You must insert 25 cents");
    initializeGame();
    startGame();
}
[download]

Where the code for errorHandler can be changed to do just about anything (log, die, warn, etc etc etc). IMO this is just as flexible as an assert subroutine. It is also faster too. Here is some code which benchmarks not only a raw OR against an assert sub, but also benchmakrs using the OR with the errorHandler as well as an example of no-op versions of both errorHandler and assert.
Read more... (6 kB)

As you can see from the results, using a basic assert subroutine is about 34% slower than the raw OR. When you add the flexibility of an errorHandler subroutine on the other end of OR, you lose only 1% (which is surely insignifagant), and gain the same flexibility an assert subroutine would have. Even when you turn off assertions and make both the assert and errorHandler routines into no-ops, the assert version is still a 21% slower (which really is useless overhead since much of that is likely just the call to assert).

Again, I like my assertions to be on at runtime, and I put them everywhere, so for me, these performance gains are a nice thing to have. But even if you don't need to worry about performance, I see little gain in flexibility over using an assert sub.

This is not a criticism of his post or his method, just a caution to the less-experienced: Premature optimization is the root of all evil .

Yes, quite true, but would you not want to make sure your always using the sharpest knife in the drawer?

I do not mean this as a means of premature optimization, but as a style, which IMO, is clear and readable, but also has the added benefit of being faster than many of the other options out there.

But then again, I hate Pepsi, and I hate Coke,... gimme some Mountain Dew anyday ;-)

-stvn

You should check out Params::Validate. It's much less verbose than what you're doing, and it's coded in XS so it's reasonably fast, though perhaps not as fast as your hard-coded tests. Your first example would like this:

my ($quarter) = validate_pos(
  @_,
  {  type => SCALAR,
     callbacks => { 'must be 25' => sub { $_[0] == 25 },
  }
);
[download]

I think that consistently using this module makes your code even more declarative.

It's much less verbose than what you're doing,...

Personally I don't see that. I have looked at Params::Validate before, and I didn't much care for it's style of parameter handling. Even if this is in XS, I would think it wouldnt be faster than a raw boolean. Already in your example there is a subroutine call (validate_pos) as well as the construction and analysis of nested hash ref (which is surely pretty fast if in XS), and then the creation of a code-ref (which I assume brings along the overhead of creating a closure, although I may be wrong in this). This seems to me to be a lot of overhead just to get to the point at which things can be checked.

I think that consistently using this module makes your code even more declarative.

I disagree, I think the more declarative approach is to use the basic boolean expression. But that is likely just a matter of personal preference/style.

-stvn

>> It's much less verbose than what you're doing,...

> Personally I don't see that. I have looked at Params::Validate before, and I didn't much care for it's style of parameter handling.

When you're validating only one parameter, the framework takes up more space than the declarative parts. But as you start adding more parameters the framework pieces grow at a much slower rate than the declarative parts. But with your code, you'll be adding another "or die" for each parameter.

Anyway, I don't think you have to use it, but I think your optimizations here are seriously premature. You should be using some sort of module for this stuff, not hand-coding it over and over and over, especially since you say you use it everywhere. You could use a source filter if you're really dead set on maximum speed.

I decided to benchmark this and see what the difference was. Here is the script...

#!/usr/bin/perl

use strict;
use warnings;

use Benchmark qw(:all);
use Params::Validate qw(:all);

sub ParamsValidateAssert {
    my ($test) = validate_pos( @_,
                        { 
                            type => SCALAR,
                            callbacks => { 'must be less than 50' => s
+ub { $_[0] < 50 }},
                        }
                    );
    return $test;
}

sub OrAssert {
    my ($test) = @_;
    ($test < 50) || die "Test is wrong";
    return $test;
}

my @nums = map { ((rand() * 100) % 50) } (0 .. 100);

cmpthese(10_000, {    
    'ParamsValidateAssert' => sub { eval { ParamsValidateAssert($_) } 
+for (@nums) },    
    'OrAssert'             => sub { eval { OrAssert($_)             } 
+for (@nums) },   
});
[download]

Here are the results...

Benchmark: timing 10000 iterations of OrAssert, ParamsValidateAssert..
+.
  OrAssert:  8 wallclock secs ( 7.02 usr +  0.01 sys =  7.03 CPU) @ 14
+22.48/s (n=10000)
ParamsValidateAssert: 97 wallclock secs (87.88 usr +  0.38 sys = 88.26
+ CPU) @ 113.30/s (n=10000)
                       Rate ParamsValidateAssert             OrAssert
ParamsValidateAssert  113/s                   --                 -92%
OrAssert             1422/s                1155%                   --
[download]

It is almost 100% faster to use OR. Again, as I said, I think my style is less verbose and more readable so I favor it becuase of that as well.

But as water points out below, it should not completely be about the speed. I am sure that Params::Validate has a number of wheels I would not want want to have to re-invent when I hand-code OR based assertions. Looking over the docs for Params::Validate, I see where this could really be a useful tool for data validation, not just of subroutine parameters, but of web queries and such. As you say it can be quite declarative and I can see how you could build some very sophisticated data validation with it. However, I do think it is overkill for subroutine params and post/pre-condition checking.

-stvn

You might like to look at Carp::Assert. Its assert function is liable to be slower than your test or die constructs, and it doesn't allow you to customize your die statements (unless you use its longer affirm routine, which is bound to be substantially slower.) I find it nice and readable, though. If I have time, I'll benchmark it later.

An idea you might like to steal from it is to append if DEBUG to your assertions, where DEBUG is a constant that you can set to 1 in development/testing and to 0 in production. That way, you have all of the benefits of assertions in development, and none of the overhead in production (along with none of the benefits.)

If I have time, I'll benchmark it later.

I was already benchmarking for the other responses, so I did it for you.

Here is the code

#!/usr/bin/perl

use strict;
use warnings;

use Benchmark qw(:all);
use Carp::Assert;

sub CarpAssert {
    my ($test) = @_;
    assert($test < 50) if DEBUG;
    return $test;
}

sub OrAssert {
    my ($test) = @_;
    ($test < 50) || die "Test is wrong";
    return $test;
}

my @nums = map { ((rand() * 100) % 50) } (0 .. 100);

cmpthese(10_000, {    
    'CarpAssert' => sub { eval { CarpAssert($_) } for (@nums) },    
    'OrAssert'   => sub { eval { OrAssert($_)   } for (@nums) },   
});
[download]

Here is the results of this:

Benchmark: timing 10000 iterations of CarpAssert, OrAssert...
CarpAssert: 11 wallclock secs ( 9.92 usr +  0.04 sys =  9.96 CPU) @ 10
+04.02/s (n=10000)
  OrAssert:  8 wallclock secs ( 7.14 usr +  0.03 sys =  7.17 CPU) @ 13
+94.70/s (n=10000)
             Rate CarpAssert   OrAssert
CarpAssert 1004/s         --       -28%
OrAssert   1395/s        39%         --
[download]

I then tried turning off Carp::Assert, and it was only 3% faster than the OR assertions.

Benchmark: timing 10000 iterations of CarpAssert, OrAssert...
CarpAssert:  9 wallclock secs ( 6.50 usr +  0.03 sys =  6.53 CPU) @ 15
+31.39/s (n=10000)
  OrAssert: 10 wallclock secs ( 6.72 usr +  0.02 sys =  6.74 CPU) @ 14
+83.68/s (n=10000)
             Rate   OrAssert CarpAssert
OrAssert   1484/s         --        -3%
CarpAssert 1531/s         3%         --
[download]

I then changed the OR assertion to use a no-op error handler (as if they were off). Here is the modified sub

sub noOpError {}
sub OrAssert {
    my ($test) = @_;
    ($test < 50) || noOpError("Test is wrong");
    return $test;
}
[download]

And surprisingly it slowed OR down

Benchmark: timing 10000 iterations of CarpAssert, OrAssert...
CarpAssert:  6 wallclock secs ( 6.54 usr +  0.00 sys =  6.54 CPU) @ 15
+29.05/s (n=10000)
  OrAssert:  7 wallclock secs ( 7.11 usr +  0.02 sys =  7.13 CPU) @ 14
+02.52/s (n=10000)
             Rate   OrAssert CarpAssert
OrAssert   1403/s         --        -8%
CarpAssert 1529/s         9%         --
[download]

An idea you might like to steal from it is to append if DEBUG to your assertions, where DEBUG is a constant that you can set to 1 in development/testing and to 0 in production. That way, you have all of the benefits of assertions in development, and none of the overhead in production (along with none of the benefits.)

Personally I do not like to turn off my assertions, I also see my debug statements as different from assertions. Besides IMO, having the if DEBUG at the end of the statement would really ruin the readability of it all.

-stvn

# warn the user of the wrong input and return
# false, leaving the calling routine dealing 
# with things
(defined($coin) && $coin == 25) || (warn("You must insert 25 cents"), 
+return 0);
[download]

# warn the user of the wrong input and return
# false, leaving the calling routine dealing 
# with things
(defined($coin) && $coin == 25) || return do {warn("You must insert 25
+ cents"); 0};
[download]

MUBA,

No offense taken at all. I usually use die or throw an exception object. So the examples with warn and such are ones I made up when writing this. I am sure they could be improved, and I agree the do does make it more readable.

-stvn