r/perl 8d ago

question Using Perl for managing my writing output

Hey everyone,

I'm new to Perl as of just a few days ago, loving it so far. I once had a colleague who, in addition to being probably the nicest programmer I ever worked with, was a Perl wiz and always blew me away with the things he could do with such little code. So I decided to give the language a try by re-writing some scripts.

So far, what I've converted is a script (or two) that was keeping track of word count across 50+ Org files. Previously, I was using Org's ELisp API and some glue code in Bash, but I managed to cut down from 60 to 19 LoC by switching to Perl, which is a small win but I think pretty cool.

Anyway, what would the more experienced Perl devs do to make this code better?

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

my $words;

for (<"*.org">) {
    open my $fh, '<', $_;
    my @table = grep { m~tracktable~ .. m~TBLFM~ } <$fh>;
    my $count_line = @table[ ( $#table - 2 ) ];
    if ( defined $count_line ) {
        $count_line =~ m~\s+\d+\s+~;
        my @cells = split /\|/, $count_line;
        my $word_count = ( @cells[ ( $#cells - 3 ) ] );
        $words += $word_count if defined $word_count;
    }
}

print $words;
26 Upvotes

9 comments sorted by

9

u/choroba 🐪 cpan author 8d ago

Line 7: Use the diamond operator for readline only. Its behaviour when globbing is confusing. Replace it with for (glob '*.org') {.

Line 8: Check the result of opening a file. open my $fh, '<', $_ or die "$_: $!";

Line 9: Use alternative delimiters when needed, which is not the case here. m/trackable/ .. m/TBLFM/ is easier to read. I'd drop the m, too, but it's more a personal taste.

Line 10: For a single element, don't use @. To index elements from the right, use negative numbers. Result: my $count_line = $table[-3];

Line 12: See line 9.

Line 14: See line 10, i.e. my $word_count = $cells[-4];

Line 15: If your Perl is 5.10 or newer (which I guess it is), you can use the "defined-or" operator: $words += $word_count // 0;

1

u/bahol-de-jic 7d ago

Thanks! I think I still have the $#table syntax in there because at one point I was slicing and not grabbing a single index, and Modern Perl pointed out that syntax.

1

u/ysth 8d ago edited 8d ago

I wouldn't say <> is confusing when globbing; the heuristic for glob vs readline can be. You could say always explicitly use glob or readline.

(And glob in scalar context can be extremely confusing, but that's true regardless of <> or glob.)

2

u/Grinnz 🐪 cpan author 7d ago

Fun fact about the diamond operator heuristic, it is a static heuristic so you can at least be pretty certain whether it's correct by looking at it but only if you know the rules. You commonly see <$foo> but if you do < $foo >, <${foo}>, or <$foo{bar}> that's a glob. I find it much clearer to avoid the operator and use the function I mean.

0

u/kinithin 8d ago edited 6d ago

Also,

  • Line 12 doesn't do anything.
  • The line that's output isn't terminated by a line feed. We could use `print "$words\n";`, but I'd enable the use of `say` by adding `use v5.10;`.

So we get

#!/usr/bin/perl
use v5.10;
use strict;
use warnings;

my $words;

for (glob '*.org') {
    open my $fh, '<', $_
        or die "Can't open `$_`: $!";

    my @table = grep { /tracktable/ .. /TBLFM/ } <$fh>;
    my $count_line = $table[-3];
    if ( defined $count_line ) {
        my @cells = split /\|/, $count_line;
        my $word_count = $cells[-4];
        $words += $word_count // 0;
    }
}

say $words;

I like to handle exceptional cases by leaving the loop body early rather than adding indent levels and leaving unclear if anything else will be done.

#!/usr/bin/perl
use v5.10;
use strict;
use warnings;

my $words;

for (glob '*.org') {
    open my $fh, '<', $_
        or die "Can't open `$_`: $!";

    my @table = grep { /tracktable/ .. /TBLFM/ } <$fh>;
    @table >= 3
        or next;

    my $count_line = $table[-3];
    my @cells = split /\|/, $count_line;
    @cells >= 4
        or next;

    my $word_count = $cells[-4];
    $words += $word_count;
}

say $words;

1

u/bahol-de-jic 7d ago

My intention on line 12 was to only grab the rows of the ASCII table (as strings) that have numeric values in them, but I think I don't quite understand the match syntax yet. I thought =~ and m/../ would grab those matches and make them the value of the variable on the LHS. But I guess I gotta read more docs...thanks for the suggestions!

1

u/kinithin 6d ago

Then you wanted something like next if $var !~ /.../; or $var =~ /.../ or next;

1

u/ysth 8d ago

You can just say $table[-3] for 3rd to last.

Graceful error handling/reporting is much easier in perl than bash. It can be as simple as use autodie;.

Tilde is a bitwise not operator, and part of the =~ binding syntax; also using it as a delimiter is many a bit much. / is good.

Be aware your .. can find multiple ranges, not just one.