Jason Balicki <[EMAIL PROTECTED]> wrote:
: Some people emailed me privately and asked that I do post
: what I've come up with to the list.
:
: I wrote this program over the course of the last week,
: starting with very little perl experience (I've modified
: others code, and small things like that) and acomplished
: a goal that prior to last week I considered unobtainable.
:
: Here is the current version. I expect to continue tweaking
: (one of the goals is to put the data in a database instead
: of a flat text file) but otherwise the program is doing what I
: want it to do.
:
: I want to say thanks again to the guys on this list
: who helped me crank out my very first, actual, useable,
: from-scratch perl program. I know it's simple, but
: I'm still proud. :)
:
: Comments are appreciated.
As John already posted, you should be using warnings and
strict in your script. Below the first line (the shebang)
place these lines. This will break your script. You'll need
to ask some questions and read the documentation to get it
running again.
#!/usr/bin/perl
use strict;
use warnings;
########################################################
# Program name: alcatel_readserial
When writing a subroutine, it should always stand alone
when finished. To do so, all input, processing, and output
needs to be done within the sub. whichline() is a good
example.
sub whichline {
if (/^.{30}\|[BNPG]\|/){
return 2;
}
else {
if (/\|[0A-Z]\|$/) {
return 1;
}
}
}
$_ is used to hold the value tested, but it was not
passed into the sub routine. Ideally we should pass a value
in. Here we use $record to hold the value while testing.
Because we used 'my', $record will be destroyed each time
we exit the sub.
sub which_line {
my $record = shift;
return 1 if $record =~ /\|[0A-Z]\|$/;
return 2 if $record =~ /^.{30}\|[BNPG]\|/;
return;
}
Let's have a look at printcallrec(). First the name.
Separate words with an underscore and spell out your words.
print_call_record() is easier to spot. Here are the first
few lines of the sub.
sub printcallrec {
my ($locbuff) = @_[0];
my ($linenum) = @_[1];
# the split function will split a string into an array using the
# specified characters as a field seperator. In this case, the "|"
# symbol is the seperator (and has to be quoted), $locbuff is the
# string to be split and @infos (couldn't think of a better name)
# is the array we'll be working with.
my (@infos) = split(/\|/, $locbuff);
In the subroutine comments we learn the following. Why
not use these descriptive names instead of unrelated or
abbreviated names. Records are normally split into fields.
What you did makes sense. I just changed the names you used.
# arguments: $locbuff (the string that contains the line data) and
# $linenum (the line number we have determined the string to be.)
sub print_call_record {
my( $line_data, $line_number ) = @_;
my @fields = split /\|/, $line_data;
The rest of this sub involves work arounds because
you are unfamiliar with everything perl can do. It looks
like you are trying to remove fields 4 and 9 from line 1
data and field 11 from line 2. Might I suggest holding
off on the split until after the line number is
determined.
sub print_call_record {
my( $line_data, $line_number, $log_file ) = @_;
my $log_data;
if ( $line_number == 1 ) {
my @fields = ( split /\|/, $line_data )[ 0 .. 8 ];
# edit date field
$fields[4] = logdate( $fields[4] );
$log_data = join ',', @fields;
} elsif( $line_number == 2 ) {
my @fields = ( split /\|/, $line_data )[ 0 .. 10 ];
$log_data = join ',', @fields;
} else {
$log_data = "\nWARNING: Invalid line number detected in " .
"print_call_record(). Expected 1 or 2. " .
"Got $line_number\n";
}
# don't clobber open LOG file handles
local LOG;
open LOG, ">>$log_file" or die qq(Can't open "$log_file": $!);
print LOG $log_data;
close LOG;
}
Printing to the log file from the sub seems like a
bad idea to me. I also don't like determining the line
number and then sending information to the sub to
determine the line number again.
if ( is_call_record( $_ ) ) {
open LOG, ">>$csv_log_file" or die qq(Can't open "$csv_log_file": $!);
print LOG format_call_record( $_ );
close LOG;
}
sub format_call_record {
my $record = shift;
my $line_mumber = which_line( $record );
if ( $line_mumber == 1 ) {
my @fields = ( split /\|/, $line_data )[ 0 .. 8 ];
# edit date field
$fields[4] = logdate( $fields[4] );
return join ',', @fields;
} elsif( $line_mumber == 2 ) {
my @fields = ( split /\|/, $line_data )[ 0 .. 10 ];
return join ',', @fields;
} else {
return
"\nWARNING: Invalid line number ($line_number)" .
" detected in print_call_record().\n";
}
}
HTH,
Charles K. Clarkson
--
Mobile Homes Specialist
254 968-8328
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>
