in reply to Improve My Code...

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.


True laziness is hard work

Replies are listed 'Best First'.
Re^2: Improve My Code...
by cheech (Beadle) on Aug 09, 2009 at 21:36 UTC
    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 =~ /...(...).../;

      is the appropriate syntax.

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


      True laziness is hard work