Puzzled about strict

[webengr]

I already knew that 'use strict;' is a Good Thing ™, but searching on Perlmonks.org has taught me to use it from the beginning, and not as an afterthought. However, I do not see an answer to my question.

I wrote the following module, not intending to re-invent the wheel, but to practice writing modules. The problem is this: when I uncomment the 'use strict;' line, there aren't any errors -- the code simply doesn't work.

This simple module provides timestamps that have configurable (more or less UNIX style) formats. The user tie's the class to a scalar; assigning a format string to the scalar results in the instance using the supplied format until it is undef'ed or assigned again. Reading from the scalar produces the timestamp string. Here's the module code:

package TimeStamp;
#use strict;
use Carp;

my @a = qw(Sun Mon Tue Wed Thu Fri Sat);
my @A = qw( Sunday Monday Tuesday Wednesday Thursday Friday Saturday);
my @b = qw(Jan Feb Mar Apr May Jun Jul Aug Sep Oct Nov Dec);
my @B = qw(January February March April May June July August September
+ October November December);
my @meridian = qw( AM PM );

# KEY TO FORMAT TOKEN CODES
# %a:short day of week, %A:long day of week, 
# %b:short month, %B:long month, %d:day of month,
# %D: date, %X:time (24 hour), %x: time (12 hour), 
# %y:short year, %Y:long year

sub a { $a[$_[6]] }
sub A { $A[$_[6]] }
sub b { $b[$_[4]] }
sub B { $B[$_[4]] }
sub d { &pad($_[3]) }
sub D { &pad(1 + $_[4]) . "/" . &pad($_[3]) . "/" . &pad($_[5] - 100) 
+}
sub X { my ($ss,$mm,$hh) = map { &pad($_) } @_; "$hh:$mm:$ss" }
sub x { my ($ss,$mm,$hh) = map { &pad($_) } @_;
        ($hh>12) ? $hh-12 . ":$mm:$ss $meridian[1]" : "$hh:$mm:$ss $me
+ridian[0]"; }
sub y { &pad($_[5] - 100) }
sub Y { $_[5] + 1900 }

# &pad() will add a leading zero to single digits. 

sub pad {
    my @result = map {($_ < 10) ? "0$_" : $_} @_;
    return wantarray ? (@result) : $result[0];
}

sub TIESCALAR {
    return (bless { fmt => \"%a %D %X" }, shift);
}

sub FETCH {
    my $obj = shift;
    my @time = localtime;
    my $fmtString = ${$obj->{fmt}};
    my @tokens = split /(%\w)/, $fmtString;
    my $result = "";
    for ( @tokens ) {
        if ( /\%./ ) {
            s/\%//;
            eval {
                $result .= &{$_}(@time);
            };
            next if ($@);
        } else {
                $result .= $_;
        }
    }
    return $result;
}

sub STORE {
    my ($obj, $fmt) = @_;
    $obj->{fmt} = \$fmt;
}

1;

And here's a test script:

#!/usr/bin/perl
use strict;
use warnings;

use TimeStamp;
my $ts;
tie $ts, 'TimeStamp';

print $ts, "\n";

$ts = "%A, %B %d, %Y";
print $ts, "\n";

$ts = "Day %d in the month of %B, the year %YAD";
print $ts, "\n";

my $ts2;
tie $ts2, 'TimeStamp';

$ts = "%D %x";
$ts3 = "%D";

print $ts2, "\n";
print $ts, "\n";

The output with 'use strict;' commented out in the module:

C:\>perl -w ts_test.pl 
Tue 10/29/02 20:07:48
Tuesday, October 29, 2002
Day 29 in the month of October, the year 2002AD
10/29/02
10/29/02 8:07:48 PM

and the output with 'use strict;' uncommented in the module:

C:\>perl -w ts_test.pl 
  
,  , 
Day  in the month of , the year AD

It seems that the module's format subroutines become inaccessable with 'use strict;' enabled.

This is ActivePerl 5.6 on Win2000 Pro.

PCS

[FamousLongAgo]

You're using the name of the subroutine as a symbolic reference, and use strict is unhappy about that. Putting a no strict in the eval block will work; a better solution might be a lookup hash:

my %lookup = (  a => \&a, 
                D => \&d, 
                X => \&X,
                x => \&x,
                d => \&d,
                     ...
         );


And the following change to the code that calls the subroutine:


$result .= &{ $lookup{$_} }(@time)


It will take a better monk than me to explain why this fails silently inside the eval block. But the solution above works ( OS 10.2, 5.8.0 ), and keeps you from using symbolic references. You can find a wonderful MJD tirade against those here, and it will make you a believer.

[converter]

...why this fails silently inside the eval block.

It "fails silently" because the eval block is working exactly as expected (see perldoc -f eval). eval BLOCK is useful for trapping exceptions at run-time (making fatal exceptions non-fatal and giving you the option to handle the exception with your own code).

[webengr]

Okay, I'm sold on the lookup hash approach, and it won't take much work to implement with the code I've already got.

Thanks for the article reference... makes perfect sense to me. And I now see why the program failed... I was throwing the error message from the eval block away (Bad programmer! No donut!)... Another day, another lesson... and that's a good thing!



PCS

[Dominus]

Says FamousLongAgo:

a better solution might be a lookup hash...
You can find a wonderful MJD tirade against [symbolic references]...it will make you a believer.

You may, however, want to see this thread, where I labeled the lookup-table approach "monkey code" and had a discussion with several people in which I pointed out that the tirade you cited does not contain any reasons why the original, symbolic reference code would really cause any problems.

In my opinion, the no strict 'refs' solution is the obvious and easy one, and so far the only arguments I have seen against it have been from superstition.

--
Mark Dominus
Perl Paraphernalia

[Anonymous Monk]

Advantages to the table lookup code:
1. Identifies which functions have a given use with less work than writing a separate package.
2. Opens you up for refactoring your code later by dynamically creating closures.
3. If you need to later, you can choose to modify the contents of the table hash dynamically without running the risk of namespace conflicts.
And yes, I have gained every one of these benefits from table-driven code. I also know how to do much the same things with typeglobs - but find the hash approach cleaner. I admit that is subjective.

And details like that are nothing compared to the crap variable and function names. I mean, conservation is good and all, but a few more letters wouldn't hurt, would it?

[Aristotle]

I'd prefer the table solution. It is monkey code indeed when it contains references to named subroutines. The solution is not to disable the stricture, but rather to build the code right into the hash and get rid of the named functions.

my %table = (
    foo => sub { ... },
    bar => sub { ... },
    baz => sub { ... },
    # ...
}

If this is not some kind of dispatch table for incoming commands, as is usually the case, but merely trying to save some keystrokes for setup code, I submit it is still the better approach. In that case, you'd accompany the construct with something like

{
    my ($n, $r);
    no strict qw(refs);
    *{$n} = $r while ($n, $r) = each %table;
}

Because this way, changing the name of a function only need be done Once And Only Once. When you change the keyname, most dependent code automatically continues working. The softref approach to saving keystrokes means you have to maintain the function names both at the sub definition as well as at the setup loop (and possibly several other locations). (You can solve this by adding a layer of indirection. Think about that approach for 10 seconds though and you'll find you're exactly back to square one. Because you'd need to store the coderefs in a hash..)

Makeshifts last the longest.

[adrianh]

In my opinion, the no strict 'refs' solution is the obvious and easy one, and so far the only arguments I have seen against it have been from superstition.

Just for arguments sake :-)

In this module we do not want any arbritary subroutine executed. We just want the "format token" subroutines executable.

At the moment this is not made explicit - it's implicit in the fact that FETCH can only execute single letter functions, and all the format token functions are single letter functions.

We could document this in POD or a comment, but my personal coding philosophy is to, as much as possible, have the code document itself.

As soon as we do:

my %lookup = (  
    a => \&a,
    D => \&d,
    X => \&X,
    x => \&x,
    d => \&d,
    ...
);

we make the subroutines we want to execute explicit in the code. This is a good thing - and hopefully not just superstition :-)

While I would immediately refactor the subs into the hash as anonymous subroutines I think that that first step is a useful one - and not just monkey code.

[converter]

Check the eval block in your FETCH method. You're doing:

next if ($@);

The code you're evaluating is violating one of the restrictions imposed by strict.pm and is causing a fatal exception, meaning that the code isn't doing what you expect it to.

update
There is only one eval expression, changed "eval blocks" to "eval block"

[djantzen]

Specifically, the error is:

"Can't use string ("a") as a subroutine ref while "strict refs" in use
+ at line [eval { $result .= &{$_}(@time); };]"

If you must, you can turn off strict ("no strict 'refs'") immediately before that eval and turn it back on immediately after.

Update: Just another thought, why not store your subroutines in a hash where the format strings are the keys? Something like:

my %method_map = (a => sub { ... }, A => sub { ... } );
my $token = 'a'; # or 'A', 'D', etc.
my $result .= $method_map{$token}->(@args);

This is strict compatible and IMO is more maintainable code.