in reply to (Ovid - Golf for golf's sake?) Re(2): Simple Rotation
in thread Simple Rotation

Golf master? Sir, I believe you have me confused with someone else :)

The main point of my reply was the reworking of the closure, and although I did intentionally skip those error checks, I was too lazy to explain why. However, since you've brought it up, let's take a closer look: 

    my $error = '';
    if ( $toggle !~ /^\d+$/ or $toggle == 0 ) {
        $error = "The first argument to &alternate must be a positive 
+integer: $toggle\n";
    }
    if ( ! @$items ) {
        $error .= "The second argument to &alternate must be an array 
+reference.";
    }
    die $error if $error;

[download]
This first test determines if $toggle is really an integer. My opinion on this is that if you want typed parameters, then go ahead and use a strongly typed language, but don't sprinkle your Perl code with ersatz type checks that work in convoluted and inefficient ways. Regexes are great for validating user input, but they're not good for patching Perl's weak typing with ad-hoc runtime error checks. If you must, write it like this: 
die "Toggle not greater than 0" unless $toggle > 0;

[download]
Then you'll get a nice argument isn't numeric warning from Perl if you accidentally passed a string, which should be sufficient.

Now look closely at the second error check. See anything wrong? The error message doesn't even correspond with the test. The error check ensures that $items is a non-empty array reference. In fact, if it's not an array reference, you won't even get to your error message complaining of this fact, as Perl would first die with the error Can't use string ("...") as an ARRAY ref. Perl already does that error check for you. But a user of your code might pass an empty array reference, see the error message claiming that he hadn't passed an array reference at all, and sit in bewilderment until he actually looked at your code, and saw the discrepency between message and test.

Despite that, you probably shouldn't even bother checking if the array is empty. Sure, it's a degenerate case, but degenerate cases are good to support when you're designing abstractions. Consider also that an array of only one element is another degenerate case. Should that too emit an error?

In your code, you've gone so far as to create a synthetic text accumulator to store multiple error messages. You'll also notice that the subroutine you named inside your error messages is different from the subroutine given. Perhaps that last criticism was below the belt, but it only serves to underscore my point that error checks aren't intrinsic goods. Error checking for the purpose of error checking is not wise. Like any other code, error checks will need to be maintained, they create the potential for new bugs, and they can adversly impact performace if poorly implemented.

So, before I add yet another error check, I ask a few questions:

Having said that, I will leave it as an excercise for the reader to verify that my code, run under strict and -w, spews warnings and errors under almost every condition that you intend for your error tests to catch.

None of this is to say that there are occasions were it's ok to not check for errors on system calls, or other external I/O. Paranoia about your outside environment is fundamental to good programming. But paranoia about the correct use of your own code isn't, and I dare say, it's even somewhat un-Perlish.

If I wanted bondage and discipline, I'd program in Java or Eiffel. As it is, I trust myself and my fellow coders enough not to feed a string into a subroutine that expects a number. I trust that if I did, I would locate that bug in relatively short order. And I trust that whatever bugs I do spend time fixing will be much more subtle and complex than the bugs that B&D programming and obsessive-compulsive error checking are designed to uncover. 

   MeowChow                                   
               s aamecha.s a..a\u$&owag.print

Replies are listed 'Best First'.
(danger - Sanity checking not evil) Re(4): Simple Rotation
by danger (Priest) on May 20, 2001 at 03:44 UTC
    Having said that, I will leave it as an excercise for the reader to verify that my code, run under strict and -w, spews warnings and errors under almost every condition that you intend for your error tests to catch.

    If someone passed 2.4 as the first argument to your routine (perhaps they calculated the value but forgot to round it)? Your code just goes ahead an uses that value (producing non-symmetric cycles), and the user of the routine has to figure out if the bug is in their code or yours (I'd be inclined to say both).

    Another possibility is if I have a reference pointing to the value I wish to use, but forget to deref that variable in the call --- a reference in numeric context resolves to the address, which is likely to be so big that no cycling will occcur, but there will be no obvious indication why it's not cycling if I can't readily spot my blunder.

    A last point is your code passes responsibility of dealing with 0 by just letting perl give an 'illegal division by zero' exception if I pass something that evaluates to zero as the first argument. This exception points to the line in the closure code, so I'll have to go and look there to find the operation in question and then backtrack to discover my calling error. Also, I won't get the error until I use the closure, though it might be nicer to know I have a bogus closure when I generate it.

    None of this is to say that there are occasions were it's ok to not check for errors on system calls, or other external I/O. Paranoia about your outside environment is fundamental to good programming. But paranoia about the correct use of your own code isn't, and I dare say, it's even somewhat un-Perlish.

    Sanity checks aren't about paranoia, they are about checking to see if everything makes sense to continue, and to provide information to the programmer/user if not. It is not a matter of being more or less Perlish. If I write a routine that multiplies two matrices, I'll do a check that they are row/column compatible rather than relying on perl to merely provide 'uninitialized' warnings to the user when things go out of bounds. Though, in such a case I'd probably issue a diagnostic warning and just return; to indicate failure. There are all kinds of useful sanity checks beyond external dependencies --- checking that input parameters are within allowable ranges is neither evil nor un-Perlish.

    Your attitude appears to be that it is the responsibility of the programmer to call your routine correctly and they should have done any sanity checking necessary prior to the call (and why should you waste your time helping them if they didn't, right?). Ovid's attitude appears to be that parameter sanity checks belong inside the routine, and wants provide some diagnostically meaningful information if they fail (although, his error message was out of sync with his code, and as you point out, the array test could have been better).

    Taking a quick stab at putting the two extremes into a module (note: I changed the spec to just accept a list rather than an array ref after the first argument, and other than changing Ovid's error checks and output, the routines are essentially as posted by both of you) we have: 

    package Cycle;
use warnings;
require Exporter;
@ISA = qw/Exporter/;
@EXPORT = qw/cycle MC_cycle/;
use strict;
use Carp;

sub cycle {  # based on Ovid's algorithm
    my $toggle = shift;
    my @items  = @_;
    unless ($toggle =~ /^\d+$/ and $toggle != 0 and @items) {
        my $me = (caller(0))[3];
        confess "Hey stupid, you are supposed to call me like this:\n"
+,
                "\t$me(positive_integer, some_list)\nNot like this:\n"
+;
    }
    my $count = 0;
    my $index = -1;
    return sub {
        $index++   if $count++ % $toggle == 0;
        $index = 0 if $index == @items;
        $items[$index];
    }
}

sub MC_cycle { # based on MeowChow's algorithm
    my ($repeat, @items, $index) = @_;
    return sub {
        $items[ int ($index++ / $repeat) % @items ];
    }
}
__END__

    [download]

    I find myself more in Ovid's camp and would prefer to see the latter of the following two error messages when I accidentally use something that evaluates to 0 as the first argument: 

    # case 1: using MC_cycle(0, @colors);
[jandrew:~]$ perl foo.pl 
Illegal division by zero at Cycle.pm line 29.


# case 2: using cycle(0, @colors);
[jandrew:~]$ perl foo.pl 
Hey stupid, you are supposed to call me like this:
        Cycle::cycle(positive_integer, some_list)
Not like this:
        Cycle::cycle(0, 'red', 'white', 'blue') called at foo.pl line 
+6

    [download]

    Personally, I'd probably issue a warning and just return; to indicate failure rather than raise an exception, but that's a whole different discussion.

[reply]
[d/l]
[select]

In Section Cool Uses for Perl