Well, if there is a suspicion that $\ is a problem (which seems unlikely, but nevertheless...), why not eliminate it as a potential issue by just explicitly using "\n" in your prints?

I looked over your code and found a lot of little red flags here and there. Closing filehandles without checking return values (ie, without checking for errors). And doing a capturing pattern match without a check to ensure it matched. One that I looked at was your handling of $previous. It appears that the variable is supposed to hold a numeric value (you do several comparisons using ==). But then early on in the script you assign "$previous = """, and later you test, "if( $previous == "" )." Luckily an empty string will equate to zero in a numeric comparison, so asking if "" == "" yields the same results as asking if "" eq "" (both evaluate to true). But it still made me queasy, so I changed it to 'eq'. It may have been better to initially set $previous to undef, and then test if it's defined().

I also made a few other changes: removed variables that were never used, set 'use autodie', eliminated some simple cases of "too much typing", and fixed your constants to be upper-cased for clarity. I reformatted the script to be easier to look at (often as I run through a script fixing its formatting I spot obvious errors that weren't so obvious when obscured by untidy formatting). I removed the useless '&' from your function calls, and eliminated the two assignments to $\. Your code also assigned to '$,', but none of your print statements printed lists (they all used concatenation with '|'), so $, wasn't doing anything for you. Removed.

I wasn't able to run your script because I don't have the database that you're querying, and even if I did, you truncated the "big SQL query" anyway (and even if you hadn't, I probably wouldn't have run it). I did verify it compiles under strictures though (had to declare $key in one loop to get it to pass strictures).

On a maintainability note, it's really unfortunate that none of the subs actually pass parameters. Everything is sort of absorbed through broader-scope osmosis. That contributes to hard-to-follow code, and the potential for bugs at a distance. If one function modifies a variable that is from a broader scope, and that variable is then used in another function, there's a side effect that took place, and it's hard to spot where. That sort of issue I didn't touch. I just left it the way it was.

Another concern is all these FIELD1, FIELD2 and so on constants. Is it really the case that the fields are so uninteresting that they couldn't be given meaningful identifiers in your list of constants? How could you keep them all straight while writing this? Did you have a cross-reference chart hand written mapping FIELD27 to the BREAST_SIZE column of your database? (ok, I made that up.) Meaningful names are easier to work with most of the time. The same goes for OUT1, OUT2, OUT3... ccr1, ccr2, ccr3, BuildCCR4, BuildCCR5, WriteCCR12.... That's the clearest use of identifier names possible? If not, think about giving them names that someone six months from now looking at your code would be able to comprehend without the use of that crossreference chart. (My opinion, which may or may not be worth anything in this case.)

So here's a somewhat modified version:

#!/usr/bin/perl use strict; use warnings; use autodie; use DBI; # Constants: use constant { FIELD0 => 0, FIELD1 => 1, FIELD2 => 2, FIELD3 => 3, FIELD4 => 4, FIELD5 => 5, FIELD6 => 6, FIELD7 => 7, FIELD8 => 8, FIELD9 => 9, FIELD10 => 10, FIELD11 => 11, FIELD12 => 12, FIELD13 => 13, FIELD14 => 14, FIELD15 => 15, FIELD16 => 16, FIELD17 => 17, FIELD18 => 18, FIELD19 => 19, FIELD20 => 20, FIELD21 => 21, FIELD22 => 22, FIELD23 => 23, FIELD24 => 24, FIELD25 => 25, FIELD26 => 26, FIELD27 => 27, FIELD28 => 28, FIELD29 => 29, FIELD30 => 30, FIELD31 => 31, FIELD32 => 32, FIELD33 => 33, FIELD34 => 34, FIELD35 => 35, FIELD36 => 36, FIELD37 => 37, FIELD38 => 38, FIELD39 => 39, FIELD40 => 40, FIELD41 => 41, }; # Capture Directory Path from Environment Variable: my $DIRECTORY = $ENV{DATADIR}; # Process Counters: my %fileCntr = ( ccr1 => 0, ccr2 => 0, ccr3 => 0, ccr4 => 0, ccr5 => 0, ); # Process Control Hashes: my %xref = (); # Process Control Variables: my ( $diag, $proc, $ndcc ) = ( 0, 0, 0 ); my $previous = ""; ## An emptry STRING. Maybe you mean '= undef'? # Claims Extract array: my @arr = (); # Accept/Parse DSS Connection String: my ( $USER, $PASS, $CONN ); if( $ENV{PSWD} =~ /(.+)\/(.+)\@(.+)/ ){ $USER = $1; $PASS = $2; $CONN = 'DBI:Oracle:' . $3; } else { die "Failed to match \$ENV{PSWD} for a " . "username, password, and DB connection.\n"; } # ALTER Date format: my $ATL = qq(ALTER SESSION SET NLS_DATE_FORMAT = 'YYYY-MM-DD'); # Database Connection: my $dbh = DBI->connect( $CONN, $USER, $PASS, { RaiseError => 1, AutoCommit => 0 } ); $dbh->do( $ATL ); # Execute ALTER session. # ######### What's missing here? ############ # my $SQL = qq( SELECT ... here is a big sql query ); # Open OUTPUT files for CCR processing: my $out_number = 1; open $_, '>', "$DIRECTORY/ccr" . $out_number++ . ".dat" for( *OUT1, *OUT2, *OUT3, *OUT4, *OUT5 ); # Redirect STDOUT to log file: open STDOUT, '>', "$DIRECTORY/ccr.log" or die "Unable to open LOG file: $!\n"; # Prepare $SQL for execution: my $sth = $dbh->prepare($SQL); $sth->execute(); # Produce out files: while (@arr = $sth->fetchrow_array) { # Direct Write of CCR1&2 records: BuildCCR12(); # Write and Wipe CCR3 HASH Table: WriteCCR3() unless ( $arr[FIELD0] == $previous ); BuildCCR3(); # Loop processing for CCR4: BuildCCR4(); # Loop processing for CCR5: BuildCCR5(); } # Print Record Counts for OUTPUT files: foreach my $key ( keys %fileCntr ) { print "$key: $fileCntr{ $key } \n"; } # Terminate DB connection: $sth->finish(); $dbh->disconnect(); # Close all output files: close $_ foreach ( *OUT1, *OUT2, *OUT3, *OUT4, *OUT5 ); sub BuildCCR12 { # Write CCR1 Table: print OUT1 $arr[FIELD6] . '|' . $arr[FIELD7] . '|' . $arr[FIELD5] . "\n"; $fileCntr{ccr1}++; # Write CCR2 Table: unless ( $arr[FIELD17] eq '###########' ) { print OUT2 ++$ndcc . "|" . $arr[FIELD0] . "|" . $arr[FIELD6] . "\n"; $fileCntr{ccr2}++; } } sub WriteCCR3 { unless ($previous eq "") { # Produce ccr3 from DISTINCT combo listing: foreach my $key ( keys %xref ) { print OUT3 $xref{ $key } . "\n"; $fileCntr{ccr3}++; } %xref = (); } } sub BuildCCR3 { # Spin off relationship: for ( my $i = FIELD8; $i <= FIELD13; $i++ ) { unless ( $arr[ $i ] == -1 ) { $xref{ $arr[FIELD0] . "|" . $arr[ $i ] } = $arr[FIELD0] . "|" . $arr[ $i ]; } } $previous = $arr[FIELD0]; } sub BuildCCR4 { # Spin off relationship: for ( my $i = FIELD26; $i <= FIELD37; $i++ ) { my $sak = $arr[FIELD0] . $arr[FIELD6] . $arr[FIELD7] . $arr[ $i ]; unless ( $arr[ $i ] eq '#######' or $arr[ $i ] eq '######' ) { print OUT4 ++$diag . '|' . $arr[FIELD0] . '|' . $arr[FIELD6] . '|' . $arr[FIELD7] . '|' . $arr[ $i ] . "\n"; $fileCntr{ccr4}++; } } } sub BuildCCR5 { # Spin off FIELD0/Procedure relationship: for ( my $i = FIELD20; $i <= FIELD23; $i++ ) { my $sak = $arr[FIELD0] . $arr[FIELD6] . $arr[FIELD7] . $arr[ $i ]; unless ( $arr[ $i ] eq '######' or $arr[ $i ] eq '####' ) { print OUT5 ++$proc . '|' . $arr[FIELD0] . '|' . $arr[FIELD6] . '|' . $arr[FIELD7] . '|' . $arr[ $i ] . "\n"; $fileCntr{ccr5}++; } } }

If it still fails, you will know that the problem wasn't related to the use of $\. But maybe you'll be lucky and find that it works better now. If I broke something kindly just keep it to yourself (just kidding :)


Dave


In reply to Re: corrupted print output by davido
in thread corrupted print output by weston2010

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • 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:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.