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>