1/ discover perltidy and use it.

2/ Don't make variables needlessly global and don't declare anything before you need to. For example, declare @dates immediately before the while loop where it is populated. @rain and @snow are not used globally at all and in fact are redeclared inside a loop - that is just plain nasty! @hlahdd is used in a very small context, but is declared globally.

3/ Use lexical file handles and good variable names generally. If FH were called $outFile the various comments describing FH's use would be redundant.

4/ Avoid redundant interpolation - push @dates, "$start"; is better written push @dates, $start;.

5/ Only the first element of @rain is used. Replace it with a scalar instead. The assignment becomes: my ($rain) = $page =~ /.../;. Ditto @snow, @depth, @hdd and @hlahdd.

6/ built in functions such as open, close, push and shift don't require parenthesis and are generally used without them.

Thanks for your suggestions, I appreciate it. This all made sense except one thing.. why when I change my pattern-match variables from arrays to scalars (@rain to $rain = $page +~ /.../ for example) do I need to have parentheses around them to get the output I want. Does it have something to do with scalar vs list data? Thanks again!

Does it have something to do with scalar vs list data?

Indeed it does. In scalar context (my $var =) a match regex returns true or false. In list context (my ($var) =) a match regex returns the list of captures. You are interested in just the contents of the first capture so:

my ($var) = $page =~ /...(...).../;
[download]

is the appropriate syntax.

Update: provide complete code line to illustrate appropriate list context usage.

---

True laziness is hard work

        while ( $start < $end ) {                        
            push @dates, "$start";                        
            chomp(@dates);                                
            $start = $start->next;                        
        }
[download]

As for this part, pulling data out of the html page:

            my @data = ($page =~ /:\s\s\s\s(\d\d)/g);
[download]

(Update: Having just seen a sample of the page delivered by that Penn State web form, I'll retract the advice about using an HTML parser. Each page of single-day data is just a sequence of fixed-width plain text lines with a '<PRE>' tag at the start of each line. Interesting approach... In any case, the method suggested below would be an improvement: it can apply generally to any set of data fields on the page, and all you have to do is manage a hash of targeted data fields.)

In addition the GrandFather's point about capturing a single value into a scalar instead of an array, your code would actually make more sense using a hash, and you could eliminate two repetitions of 7 lines of code by looping over hash keys "rain snow depth". (A hash of hashes can also store the regex to be matched and format to be used for each key, as well as the extracted values.) That way, the next time you change something that affects all three fields, you only have to make one edit instead of three. (DRY: Don't Repeat Yourself.)

Anyway, I agree with merlyn's point that this is an offensive amount of web traffic to get a small amount of data that should be more easily (and reliably) accessible, in a more usable tabular form, by some other means.

The code below is used to create a list of dates from 18960101 to 20090717 using Date::Simple, access and submit a form using Mechanize

Just because you can scrape something doesn't mean you should. Please learn the difference.

According to the site's Statement of Purpose,

We now have the capability to let the user choose a date of interest and get the weather statistics from that date. The entire record of daily observations is now available. Graphical representation of the data, especially long-term data, is included and is being augmented.

cheech's use of the site is arguably reasonable. And maybe his hits will help them justify funding for more bandwidth or server horsepower--so he could be doing them a favor.

Respectfully,
Matt

It says "the user choose a date". If I was writing that, I'd have the expectation that humans would be entering a date, not a mechanized attack of all dates.

True, there's nothing that forbids what he wants to do, but it's also a matter of courtesy. Don't go hammering someone else's machine without permission, and I wouldn't consider that paragraph permission, just description.

-- Randal L. Schwartz, Perl hacker

The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119.

Maybe they'll think about repackaging the data. Its static, organized by date, and a CGI interface isn't really the best way to deliver it.

it's unethical, unless you have written permission from the owners

Agreed. Perhaps these could be updated to reflect the position? •Re: Swimsuit issue, •Re: •Re: Swimsuit issue, •Re: •Re: •Re: Swimsuit issue, and •Re: Re: •Re: •Re: Swimsuit issue.

unless you have written permission from the owners.

Go figure, I do. Read the op again, you didn't get what I was asking for I guess..

FYI, it was made to be scraped. Please don't crusade at every opportunity.

Also

while (@dates) {
    ......
    shift(@dates);
}
[download]

better to write as:

foreach my $date (@dates) {
    .....
}
[download]

I initially used foreach(@array){...} instead of while(@array){ ... shift(@array)} but changed it because I was getting booted from the server after a few hundred iterations, and was planning on restarting the loop from the current iteration. As it turned out, the problem was on my end of the connection, so I never needed to implement that and just never went back to foreach. Why though is it superior to while in this case?

chomp(@dates);

Only if they are pitted dates! (Otherwise, it's off to the dentist.)

Looks like you've got some Perl poetry there :)

I've chosen a data driven design. I've extracted all the field options to one place $fields: defining the regexp to parse the string, possible fixups and the fortran output format.

I've added Getopt::Long to let the user pick a specific range of dates. All of the configurable information is at the start of the script. I'd eventually pull the parsing into a module with a callback to print the output lines, so that the script just contains the part of user interest.

WWW::Mechanize::Cached will build a nice local-side cache, which is handy when you realize half-way through a huge scrape run that you need to grab an extra field. I query if the last request came out of cache, and skip sleeping before the next request.

#!/usr/bin/perl

use strict;
use warnings;

use WWW::Mechanize::Cached;
use Date::Simple::D8 (':all');
use Fortran::Format;
use Getopt::Long;

#configuration:
my $start_date = '19400101';
my $last_date  = '20090718';
my $filename   = "c:/perl/scripts/496/hla.txt";
my $root_url   = "http://bub2.meteo.psu.edu/wxstn/wxstn.htm";
my $help       = 0;

my $result = GetOptions(
    "start=s" => \$start_date,
    "end=s"   => \$last_date,
    "help"    => \$help,
);
my $usage = <<EOS;
$0 - Parse $root_url from $start_date to $last_date
Options:
 --start=19400101
 --end=20090718
EOS
die $usage if ( $help or !$result );

my $fields = {
    depth => {
        regexp => qr/Rain or Liquid Equivalent\s+:\s+(\S*)/,
        format => "I2.1,6X",
        fix    => { TRACE => 99, '(N/A)' => 99, '0' => 99 },
    },
    rain => {
        regexp => qr/Snow and\/or Ice Pellets\s+:\s+(\S*)/,
        format => "F4.2,6X",
        fix    => { TRACE => '0.00' },
    },
    snow => {
        regexp => qr/Snow Depth\s+:\s+(\S*)/,
        format => "F4.2,6X",
        fix    => { TRACE => '0.00' },
    },
    hdd => {
        regexp => qr/Degree-Days\s+:\s+(\S*)/,
        format => "I2.1,6X",
    }
};

sub print_output {
    my ( $date, $data, $fortran ) = @_;

    my @data    = @$data;
    my %fortran = %$fortran;

    my $output =
      join( " ", $date, @data[ 0 .. 2 ], @fortran{qw( depth hdd rain s
+now )} )
      . "\n";

    print $output;
    print OUTPUT $output;
}

#### end configuration

# Open file for writing
open( OUTPUT, '>', $filename ) or die "open: $!\n";

# Initiate browsing agent
my $mech = WWW::Mechanize::Cached->new( keep_alive => 1 );

# Create date list
my $date     = Date::Simple::D8->new($start_date)->prev;
my $end_date = Date::Simple::D8->new($last_date);

$mech->get($root_url);
while ( $date->next <= $end ) {

    # Submit the first form
    my $resp = $mech->submit_form(
        form_number => 1,
        fields      => { dtg => $date }
    );

    # Download the resulting page, text only, and scrape for data
    my $page = $mech->content( format => 'text' );
    my @data = ( $page =~ /:\s\s\s\s(\d\d)/g );
    my %fortran;
    foreach $field ( keys %$fields ) {
        my $regexp = $fields->{$field}->{regexp};
        my $format = $fields->{$field}->{format};
        my $fix    = $fields->{$field}->{fix} || {};

        #parse page for this field
        my ($parsed) = $page =~ /$regexp/;

        #fix field
        foreach my $key ( keys %fix ) {
            $parsed = $fix{$key}
              if $parsed eq $key;
        }

        # Format the output for Fortran analysis
        chomp( my $f = Fortran::Format->new($format)->write($parsed) )
+;
        $fortran{$field} = $f;

    }

    # Prepare output for screen and file
    print_output( $date, \@data, \%fortran );

    sleep .1 unless $mech->is_cached();
    $mech->back();
}    # Exit the loop

# Close the written file
close(FH);
[download]

yes/no/maybe-so? What'd ya think?

Specifically, I don't like how I treat @data differently from the other fields. What is in @data? Maybe all of the $fields should be grabbed as arrays, with a subfield indicating the desired indices?