steph_bow has asked for the wisdom of the Perl Monks concerning the following question:


Dear Wise and Serene Monks,

I have made a code that gives the expected result but gives a lot of warnings, so it means my code shoud be cleaner
Could you tell me how ?

Here is the start file :
08:48:45;"";"";;""; 08:49:43;"DAL11";"08:47";527;"08:50";530 08:50:41;"";"";;""; 08:51:40;"";"";;""; 08:52:38;"";"";;""; 08:53:36;"ACA879";"08:51";531;"08:53";533 08:54:35;"";"";;""; 08:55:33;"SHT8K";"08:48";528;"08:55";535 08:56:31;"";"";;""; 08:57:30;"DLH9FM";"08:53";533;"08:57";537

Here is my code :
#!/usr/bin/perl use strict; use warnings; use diagnostics; ################# IN WHICH DIRECTORY WE ARE ######################## my $Current_Dir = `pwd`; print STDOUT "the current directory is $Current_Dir"; ##################################################################### ################### OPEN THE FIRST INFILE ############################ # open the first file # do not forget the "" to declare my ${first_file} = my ${first_file} = "$ARGV[0]"; open(INFILE,"<${first_file}") or die "Can't open ${first_file} : $!"; ################################################################# ################### OPEN THE OUTFILE ############################ # name of the OUTFILE # never put a \n at the end of the OUTFILE name otherwise it does not +create the output my ${outfile_name} = "Analysis_${first_file}"; # to open the file # OUTFILE is the name of the HANDLE in this case open (OUTFILE, ">${outfile_name}") or die "Can't open ${outfile_name +}: $!"; ################################################################## while (<INFILE>){ chomp($_); my @Parts = split(";"); my $aircraft_id = $Parts[1]; my $ATO_min = $Parts[3]; my $CTO_min = $Parts[5]; my $ATO = $Parts[2]; my $CTO = $Parts[4]; my $delay_ATO_CTO_min; if (($CTO =~ /(\d\d):(\d\d)/) & ($ATO =~ /(\d\d):(\d\d)/)){ $delay_ATO_CTO_min = $CTO_min-$ATO_min; } else{ $delay_ATO_CTO_min = ""; } my $flight_big_delay; # condition on the delay if ($delay_ATO_CTO_min > 10){ $flight_big_delay = $aircraft_id; print OUTFILE "$aircraft_id;$ATO_min;$CTO_min;$delay_ATO_CTO_m +in;$flight_big_delay\n"; } else{ print OUTFILE "$aircraft_id;$ATO_min;$CTO_min;$delay_ATO_CTO_m +in\n"; } } close INFILE; close OUTFILE;

Here is my output file
"";;; "DAL11";527;530;3 "";;; "";;; "";;; "ACA879";531;533;2 "";;; "SHT8K";528;535;7 "";;; "DLH9FM";533;537;4

And here are the warnings:
Argument "" isn't numeric in numeric gt(>) at Monks_Treatment.pl (that +'s the name of the program) line 60, <INFILE>
update : the update concerns both the input and output since the original post data copied from wordpad, not from excel, for the input data as well as the output data

Replies are listed 'Best First'.
Re: warnings unexpecetd
by shoness (Friar) on Aug 07, 2007 at 09:37 UTC
    If the "else" branch in line 51 is taken, your variable $delay_ATO_CTO_min will be "" when you reach line 60 and you'll get the warning you mention. Please consider:
    if (($delay_ATO_CTO_min ne "") && ($delay_ATO_CTO_min > 10)) {
    Also, the warning for line 48:
    Use of uninitialized value in concatenation (.) or string at ./630978. +pl line 66, <INFILE> line 1 (#1)
    is another clue that many of your values aren't initialized correctly. Many lines in your input file don't have any "aircraft information", just a timestamp. You should skip those lines (I guess). I think you also "split" on ";" when you should split on whitespace. Consider this instead...
    my @Parts = split('\s+'); next if (scalar @Parts != 6);
    Good luck!
Re: warnings unexpecetd
by citromatik (Curate) on Aug 07, 2007 at 09:43 UTC

    The warning that you comment is due to:

    $delay_ATO_CTO_min = "";

    After that assignment you try to use $delay_ATO_CTO_min as a number:

    if ($delay_ATO_CTO_min > 10){

    Nevertheless, the script you posted doesn't seem to parse the input file you provided (the fields in the input file are not separated by ";", you don't check for "empty" lines (e.g. lines with only the time), etc...). Here is a version of your script with some of these problems corrected:

    use strict; use warnings; while (<DATA>){ chomp($_); my ($aircraft_id,$ATO,$ATO_min,$CTO,$CTO_min) = (split /\s+/)[1,2,3, +4,5]; next if (!defined $ATO); my $delay_ATO_CTO_min = 0; if ((defined $CTO && $CTO =~ /\d\d:\d\d/) && (defined $ATO && $ATO +=~/\d\d:\d\d/)){ $delay_ATO_CTO_min = $CTO_min-$ATO_min; } my $flight_big_delay; # condition on the delay if ($delay_ATO_CTO_min > 10){ $flight_big_delay = $aircraft_id; print "$aircraft_id;$ATO_min;$CTO_min;$delay_ATO_CTO_min;$flight_b +ig_delay\n"; } else{ print "$aircraft_id;$ATO_min;$CTO_min;$delay_ATO_CTO_min\n"; } } __DATA__ 08:48:45 08:49:43 DAL11 08:47 527 08:50 530 08:50:41 08:51:40 08:52:38 08:53:36 ACA87 08:51 531 08:53 533 08:54:35 08:55:33 SHT8K 08:48 528 08:55 535 08:56:31 08:57:30 DLH9F 08:53 533 08:57 537

    Outputs:

    DAL11;527;530;3 ACA87;531;533;2 SHT8K;528;535;7 DLH9F;533;537;4

    citromatik

Re: warnings unexpecetd
by FunkyMonk (Bishop) on Aug 07, 2007 at 09:31 UTC
    There's two main problems, that I can see:

    1. You're splitting the input on ";", but your input doesn't contain any!

    2. You're taking two times, formatted like "09:50" and subtracting one from the other, presumably to get their difference in minutes. Perl won't do that for you automatically, so use a funtion something like

      sub time2mins { my $time = shift; my $mins = ''; $mins = $1 * 60 + $2 if $time =~ /(\d\d):(\d\d)/; return $mins; }

      To convert before you subtract.

    updated: Need more tea

Re: warnings unexpecetd
by Anno (Deacon) on Aug 07, 2007 at 09:54 UTC
    As FunkyMonk has remarked, the format of your input file (primarily space-separated) doesn't match the way you split each line into fields (split on ";"), so it is not surprising that some of the fields come out undefined.

    Moreover, you say:

    Here is my output file

    DAL11 527 530 3 ACA87 531 533 2 SHT8K 528 535 7 DLH9F 533 537 4
    ...but the typical print statement in your program is
    print OUTFILE "$aircraft_id;$ATO_min;$CTO_min $delay_ATO_CTO_m";
    That would print a semicolon-separated output, not the space-separated one you present. You need to reconcile your separators.

    Anno