[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 463
  • Last Modified:

Help improving the output using the Perl Business::ISBN Module

This is the script I wrote:

#!/usr/bin/perl

use strict;
use Encode qw(encode decode);
use Time::HiRes qw(gettimeofday);
use File::Copy;
use Business::ISBN qw( valid_isbn_checksum );  


my $t0 = gettimeofday;

##

system ("echo processing part four of five \.\.\.");
our @files = glob("ean.txt");      
foreach my $file(@files) {

                      open FILE, '<:encoding(UTF-8)', $file or warn "Can't open $file: $!";  
                      open PARSED, '>:encoding(UTF-8)', ($file . "_invalid.txt") or warn "Cannot open file for write: $!";  

while (<FILE>) {

foreach ($_) {

print $_ . valid_isbn_checksum($_) . "\n";


}

print PARSED;
}
}


close FILE;
close PARSED;

And it compiles with no problem. The output that's sent to the screen is:

$ perl Edit3
processing part four of five ...
9781481412063
1
9781442369405
1
9781476762975
1
97822222211111

The valid_isbn_checksum() function returns 1 if the ISBN is valid and 0 or undef if the ISBN is invalid. Of these 4 ISBNs, the first three of them are valid and the last one isn't.

And the file output has just a print out of the ISBN:

9781481412063
9781442369405
9781476762975
9782222221111

I would like just to see just the invalid ones and having the file output that would return the invalid number and "is invalid", like "9782222221111 is invalid"; thanks
0
hadrons
Asked:
hadrons
  • 2
  • 2
2 Solutions
 
ozoCommented:
print "$_ isinvalid\n" unless valid_isbn_checksum($_);
0
 
hadronsAuthor Commented:
The results stay the same, the only difference is that the output didn't reach the screen, but in the file all the ISBN printed whether they were invalid or not.

I even tried adding this:

if (!/1/) {

print $_;
}

And there was no change in the results
0
 
ozoCommented:
If the results stay the same, what did you change in the code?
If you want to change what is printed to PARSED
change
print PARSED;
to
print PARSED "$_ is invalid\n" unless valid_isbn_checksum($_);
0
 
FishMongerCommented:
The adjustment that ozo gave should fix your printing issue, but I'd like to point out some issues that you should look at and consider fixing.

system ("echo processing part four of five \.\.\.");
Perl has a print function, so using a system call to execute the echo function is odd and not what you should be doing.
print "processing part four of five ...";

Open in new window

our @files = glob("ean.txt");
foreach my $file(@files) {
Why are you using 'our' which declares a global var instead of using 'my' to declare the more appropriate lexical var?

I'll assume that your actual script is using a pattern rather than hard coding a single filename in the glob statement.  Otherwise, using glob would not make any sense.

open FILE, '<:encoding(UTF-8)', $file or warn "Can't open $file: $!";  
open PARSED, '>:encoding(UTF-8)', ($file . "_invalid.txt") or warn "Cannot open file for write: $!";
Outputting an error message if the open call fails is good, however you should use die instead of warn.  If the open call fails continuing on and attempt the parsing of the file's contents won't be fruitful and doesn't make any sense to try.

Current Perl coding practices/standards recommend using the 3 arg form of open and a lexical var for the filehandle.
foreach ($_) {
$_ is a scalar, not an array so looping over a single element doesn't make any sense.
0
 
hadronsAuthor Commented:
It's clear from my feedback that I'm not a trained programmer, but I do use Perl heavily because its so useful and I'm largely self-taught. I tend to try different things even after submitting a question and if some of the practices seem odd its because I tend to fall back on what previously worked.

I did rework the script using both of your recommendations and it works just how I want it:

#!/usr/bin/perl

use strict;
use Encode qw(encode decode);
use Time::HiRes qw(gettimeofday);
use File::Copy;
use Business::ISBN qw( valid_isbn_checksum );  


my $t0 = gettimeofday;

##

print ("echo processing part four of five \.\.\.");
my @files = glob("ean.txt");      
foreach my $file(@files) {

                      open FILE, '<:encoding(UTF-8)', $file or warn "Can't open $file: $!";  
                      open PARSED, '>:encoding(UTF-8)', ($file . "_invalid.txt") or warn "Cannot open file for write: $!";  

my @eans = <FILE>;
foreach my $eans (@eans){
print "$eans is invalid\n" unless valid_isbn_checksum($eans);
print PARSED "$eans is invalid\n" unless valid_isbn_checksum($eans);
}
}
close FILE;
close PARSED;

I do need the glob function because I will be working with files with a specific pattern (I just used eans.txt because I wanted just one test)
0

Featured Post

Free Tool: Port Scanner

Check which ports are open to the outside world. Helps make sure that your firewall rules are working as intended.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 2
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now