Perl: Three parts of code works independently of each other, when mixed, the script fails

Question

I have the following script to make pdf from jpgs, tifs, and pngs. Each part (starting with #indepedently works OK) works fine. In other words, when I comment the JPG part, the script works for PNG, when I comment JPG and PNG part, it works for TIFs. However, it fails to work for all the extensions: JPG, PNG and TIF. I want the script to look for JPGs in a folder. If they are found, a pdf should be created (and only then, I do not want an empty pdf) and the script should exit. If there are no JPGs, the script should look for PNGs. If they are found, a pdf should be created and the script should exit. If there are no PNGs, the script should look for TIFs. If they are found, a pdf should be created and the script should exit. The script is as follows:

#!/usr/bin/perl

use PDF::API2;
use strict;
use warnings;

# assume all just one directory path
my $folder = join(' ', @ARGV);

# deal with dos paths
$folder =~ s|\\|/|g;

$folder =~ s|/$||;

my $pdf_file = $folder . '.pdf';

die "Not a folder!\n" unless -d $folder;
die "There's already a pdf of that name!\n" if -f $pdf_file;

my $pdf = PDF::API2->new;

#indepedently works OK - JPG
foreach my $file ( glob "$folder/*.jpg" ) {
    my $jpg = $file;
    my $image = $pdf->image_jpeg($jpg);
    my $page = $pdf->page();
    $page->mediabox(0,0,$image->width, $image->height);
    $page->trimbox(0,0,$image->width, $image->height);
    my $gfx = $page->gfx;
    $gfx->image($image, 0, 0);
}

$pdf->saveas($pdf_file);

#indepedently works OK - PNG
foreach my $file ( glob "$folder/*.png" ) {
    my $png = $file;
    my $image = $pdf->image_png($png);
    my $page = $pdf->page();
    $page->mediabox(0,0,$image->width, $image->height);
    $page->trimbox(0,0,$image->width, $image->height);
    my $gfx = $page->gfx;
    $gfx->image($image, 0, 0);
}

$pdf->saveas($pdf_file);

#indepedently works OK - TIF
foreach my $file ( glob "$folder/*.tif" ) {
    my $tif = $file;
    my $image = $pdf->image_tiff($tif);
    my $page = $pdf->page();
    $page->mediabox(0,0,$image->width, $image->height);
    $page->trimbox(0,0,$image->width, $image->height);
    my $gfx = $page->gfx;
    $gfx->image($image, 0, 0);
}

$pdf->saveas($pdf_file);

Answer 1

Here are the pertinent guts for a slightly different approach that cuts down on your code duplication.

We create a dispatch/lookup table to externalize the dissimilar bits of code, keyed off the file extensions you're interested in. In the outer loop, we iterate over those extensions in the order you want (which, luckily, happens to be lexical order in this example). In the inner loop, we filter out files for the current extension and then apply the page creation logic. Finally, we only save the file if the document actually contains some pages.

my $pdf = PDF::API2->new;

my %dispatch = (
    jpg => sub { $pdf->image_jpeg(@_) },
    png => sub { $pdf->image_png(@_) },
    tif => sub { $pdf->image_tiff(@_) },
);

my @files = glob("$folder/*");

for my $ext (sort(keys(%dispatch))) {
    my $count;

    for my $file (@files) {
        next unless $file =~ /\.$ext$/;

        my $image = $dispatch{$ext}->($file);
        my $page = $pdf->page;
        $page->mediabox(0, 0, $image->width, $image->height);
        $page->trimbox(0, 0, $image->width, $image->height);
        my $gfx = $page->gfx;
        $gfx->image($image, 0, 0);

        $count++;
    }

    last if $count;
}

if ($pdf->pages) {
    $pdf->saveas($pdf_file);
}

Answer 2

This code more or less implements what I suggested in my comment:

#!/usr/bin/perl

use PDF::API2;
use strict;
use warnings;

sub convert_jpgs
{
    my($pdf, $folder) = @_;
    my $count = 0;
    foreach my $file ( glob "$folder/*.jpg" )
    {
        my $jpg = $file;
        my $image = $pdf->image_jpeg($jpg);
        my $page = $pdf->page();
        $page->mediabox(0,0,$image->width, $image->height);
        $page->trimbox(0,0,$image->width, $image->height);
        my $gfx = $page->gfx;
        $gfx->image($image, 0, 0);
        $count++;
    }
    return $count;
}

sub convert_pngs
{
    my($pdf, $folder) = @_;
    my $count = 0;
    foreach my $file ( glob "$folder/*.png" )
    {
        my $png = $file;
        my $image = $pdf->image_png($png);
        my $page = $pdf->page();
        $page->mediabox(0,0,$image->width, $image->height);
        $page->trimbox(0,0,$image->width, $image->height);
        my $gfx = $page->gfx;
        $gfx->image($image, 0, 0);
        $count++;
    }
    return $count;
}

sub convert_tifs
{
    my($pdf, $folder) = @_;
    my $count = 0;
    foreach my $file ( glob "$folder/*.tif" )
    {
        my $tif = $file;
        my $image = $pdf->image_tiff($tif);
        my $page = $pdf->page();
        $page->mediabox(0,0,$image->width, $image->height);
        $page->trimbox(0,0,$image->width, $image->height);
        my $gfx = $page->gfx;
        $gfx->image($image, 0, 0);
        $count++;
    }
    return $count;
}

for my $folder (@ARGV)
{
    # deal with dos paths
    $folder =~ s|\\|/|g;
    $folder =~ s|/$||;

    if (! -d $folder)
    {
        warn "$folder: not a folder! - skipping\n";
        next;
    }

    my $pdf_file = $folder . '.pdf';
    if (-f $pdf_file)
    {
        warn "There's already a pdf $pdf_file! - skipping\n";
        next;
    }

    my $pdf = PDF::API2->new;
    if (convert_jpgs($pdf, $folder) != 0 ||
        convert_pngs($pdf, $folder) != 0 ||
        convert_tifs($pdf, $folder) != 0)
    {
        $pdf->saveas($pdf_file);
    }
    else
    {
        warn "No image files in $folder\n";
    }
}

It compiles; it has not been tested. It changes the interface; it assumes that you might want to process several folders with a single invocation and assumes you type the folder names accurately on the command line, inside double quotes if the names contain spaces, etc. Thus, if the script is c3po.pl (for 'correct three Perl operations' — nothing to do with a new hope, of course, or an awakening force), then you could run, perhaps:

perl c3po.pl $HOME/Images/*

and it will process the documents in each of the subdirectories under $HOME/Images.

Note how each subroutine takes the working code, reads two arguments into local (my) variables — the parentheses are important — and creates a new $counter which it initializes, increments and returns. Otherwise, the code in the functions is substantially unchanged from the code you provided.

Note how the driver code (the for loop over the argument list) uses warn rather than die (it isn't necessarily a good idea to stop processing just because there's a minor problem — though it is often a good strategy to exit early), and how the warning messages contain the name of the object that causes trouble (the directory name or the file name). This makes it much easier to debug — you know what the program was looking for and failed to find.

If you wanted to get fancy, you'd pass a reference to the relevant function from the PDF::API2 package, and the relevant suffix, so that you could have a single function handling all three image types. That's slightly more complex than what's shown but avoids more repetition — laziness is good and repetition is bad. However, if you're struggling with functions in the first place, that's out of scope for a sensible, helpful answer.

Answer 3

You need to have a "new" PDF every time you want to save. Just add

$pdf = PDF::API2->new;

Before the second and third foreach loop.