Unreadable Perl Code

grantm on 2006-03-20T08:52:47

I have a replication process which applies updates. Usually the updates are incremental 'patches' sometimes they are full copies. The incoming queue of updates might look like this:

my @files = qw(
    00000.diff
    00001.diff
    00002.diff
    00003.copy
    00004.diff
    00005.diff
    00006.copy
    00007.diff
);

If there are one or more full copies in the queue, there is no point processing any updates before the last full copy. So what I needed was an algorithm to reduce the above example to just the last two files.

My first thought was that I ought to be able to do it with some form of grep using the flip-flop operator. Alas, my initial attempts were not successful so I wrote a loop that popped things off one array and unshifted them onto another - breaking out of the loop if a full copy file was found. It wasn't complicated code but it took more lines than I'd have liked and it needed a one-line comment explaining what the loop was doing (perhaps, in retrospect, I should have put it into a subroutine).

I later realised what I was doing wrong in my first attempts and came up with this version:

my $i = 0;
@files = reverse grep !$i++../\.copy$/, reverse @files;

Having to use $i was annoying but I couldn't think of another way to code an expression that was true the first time it was evaluated and false from then on.

Anyway, I ran this version past a couple of my colleagues and while they were intrigued, they felt it was basically unreadable and didn't want to be saddled with maintaining it. So I 'committed' it here instead :-)

---

My shot

bart on 2006-03-20T14:10:57

The only assumption I make is that the files in the list are in sequential order.

my @keep;
foreach (@files) {
    @keep = () if /\.copy$/;
    push @keep, $_;
}

Re:My shot

grantm on 2006-03-21T18:34:00

Of all the suggestions, this one definitely gets my vote for clarity. I did think along the lines of discarding accumulated filenames whenever a '.copy' was encountered but at the time I had grep blinkers on and it can't be done with grep.

before_incl

stvn_skuo on 2006-03-20T15:10:22

Perhaps try the List::MoreUtils module?

use List::MoreUtils qw/before_incl/;
my @keep =
     reverse
     before_incl { /\.copy\z/ }
     reverse @files;

Unreadable Perl Code

Eric Wilhelm on 2006-03-20T23:18:37

...So what I needed was an algorithm to reduce the above example to just the last two files....

Or, at least, you want to get the last index of a /\.copy$/ match from the list.

my $index = (
    grep({$files[$_] =~ m/\.copy$/}
        reverse(0..$#files)
    )
)[0];
splice(@files, 0, $index);

List::Util improves this

Aristotle on 2006-03-21T12:29:58

use List::Util qw( first );
my $index = first { $files[ $_ ] =~ m/\.copy$/ } reverse 0 .. $#files;
splice @files, 0, $index if defined $index;

No need for the flip-flop

Aristotle on 2006-03-21T12:18:11

my $seen_full_copy;
@files = reverse grep { not( $seen_full_copy ||= /\.copy$/ ) } reverse @files;

The double reverse is still unsightly though.

There's no fold in List::Util or List::MoreUtils

mary.poppins on 2006-03-28T06:25:51

There's a "reduce" in List::Util, but its interface is fairly horribly broken.

Anyway, here's a fold-using implementation:


use strict;

sub fold {
    my $accum = shift;
    my $code = shift;
    return $accum unless @_;
    return fold( $code->( $accum, shift ), $code, @_ );
}

my @files = qw(
        00000.diff
        00001.diff
        00002.diff
        00003.copy
        00004.diff
        00005.diff
        00006.copy
        00007.diff
        00008.diff
);

use Data::Dumper;
sub foldee {
        my ( $accum, $item ) = @_;
        return [$item, []] if ( $item =~ /\.copy$/ );
        return $accum unless defined $accum->[ 0 ];
        return [ $accum->[ 0 ], [@{$accum->[1]}, $item]];
}
print Dumper(fold( [undef, []], \&foldee, @files));