in reply to Review of first real code
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:
$filename = $hash{"LIS_FILE"} if($node eq "sam"); $filename = $hash{"LIS_FILE_TEST"} if($node eq "cad2");
or better yet:my $filename = $hash{LIS_FILE}; $filename = $hash{LIS_FILE_TEST} if $node eq 'cad2';
But see above. You shouldn't be doing it for this anyway.my $filename = $node eq 'sam' ? $hash{LIS_FILE} : $hash{LIS_FILE_TEST} +;
my ($arg, $arg2, $argument, $another_arg, $yet_another, $and_another, $still_more, $last_argument ) = @_;
is not very helpful. Forget the comment and write that as my $prospect_id = shift; (but see my last comment as well.)# prospect_id my $prosp_id = shift @_;
is downright ugly. There are many idioms for file slurping. Choose one. $line = do {local $/; <IN>}; for instance.$line = <IN>; while(<IN>){ $line .= $_; }
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.";
|
|---|