comment on

This code is difficult to read. You could improve it a lot with better formatting. Very long lines should be avoided. Break them up into shorter lines. Use here documents or string concatenation to get long strings under control. Be consistent.

Some notes:

You should use strict;.
Consider turning warnings on too.

$hash is a poor name for a variable in a real program.

You have a lot of code like:

$filename = $hash{"LIS_FILE"} if($node eq "sam");
$filename = $hash{"LIS_FILE_TEST"} if($node eq "cad2");
[download]

It isn't easy to read. Since you are using different config files depending on the node, there is no real reason to use different hash keys as well.
You use those literals, "sam" and "cad2", many times throughout your code. What if you need to run it on "elmer" or "cad3" next week?
As you are setting variables immediately after declaring them with my, there is no reason to initialize them to "" first. Just set them to what you want.
```
my $filename = $hash{LIS_FILE};
   $filename = $hash{LIS_FILE_TEST} if $node eq 'cad2';
[download]
```
or better yet:
```
my $filename = $node eq 'sam' ? $hash{LIS_FILE} : $hash{LIS_FILE_TEST}
+;
[download]
```
But see above. You shouldn't be doing it for this anyway.

Your update_record sub takes 8 arguments. That's a lot. You might want to consider using another data structure that holds that information. In any case, using shift repeatedly to pull them off is ugly. Consider something like:

my ($arg,         $arg2,        
    $argument,    $another_arg,
    $yet_another, $and_another, 
    $still_more,  $last_argument ) = @_;
[download]

Commenting like:

# prospect_id
my $prosp_id = shift @_;
[download]

is not very helpful. Forget the comment and write that as my $prospect_id = shift; (but see my last comment as well.)

Consider the use of single quoted strings where you don't actually need variable interpolation. 'nsmail@sam.paychex.com' is better than "nsmail\@sam.paychex.com"

Consider refraining from quoting hash keys. $hash{LIS_FILE} is easier on the eyes than $hash{"LIS_FILE"}

Consider relying on default behavior. Why use next if($_ =~ /^#/); when next if /^#/; will do?

This construct:

$line = <IN>;
while(<IN>){
    $line .= $_;
}
[download]

is downright ugly. There are many idioms for file slurping. Choose one. $line = do {local $/; <IN>}; for instance.

The diamond operator works intuitively in list context. push(@msg,$_) while(<MSG>); does the same thing as @msg = <MSG> only slower and more verbosely.

Don't use: my $date = `date`;... perldoc -f localtime.

Although you are a beginner, the program is reasonably large and making it all work together must have been quite a feat. I applaud the effort.

You did, however, make it harder than it needed to be in many ways and the code won't be very easy to maintain. Now that you have it working, I suggest that you write it again. You might think I'm crazy for suggesting it but I bet you would get a lot out of it. Come back with the next version and ask again. :-)

-sauoq
"My two cents aren't worth a dime.";

In reply to Re: Review of first real code by sauoq
in thread Review of first real code by SPIDEY

Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!

Titles consisting of a single word are discouraged, and in most cases are disallowed outright.

Read Where should I post X? if you're not absolutely sure you're posting in the right place.

Please read these before you post! —

Posts may use any of the Perl Monks Approved HTML tags:

a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr

You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)

	For:		Use:
	&		`&`
	<		`<`
	>		`>`
	[		`[`
	]		`]`

Link using PerlMonks shortcuts! What shortcuts can I use for linking?

See Writeup Formatting Tips and other pages linked from there for more info.