If that doesn't fire, you end up doing four more "if" tests on every iteration, and then deleting an element from an array (which probably doesn't do what you think it does -- check "perldoc -f delete").next if( $options{"unoccupied"} eq "0" && $_[6] eq "unoccupied\n" );
So there are ways to reduce the overall amount of work that perl is doing in the loop while still getting things done just as well (or even better). I don't know how much execution time would be saved, but I think the following version is at least a bit cleaner and not any harder to maintain than your original.
As mentioned previously, you haven't shown how you initialize some of the variables in the posted snippet, so I'm going to guess that your "$planets" value was intialized to the number of elements in the "@data" array (i.e. your for loop covers every array element).
Another thing I have to guess about is that elements in @data either have 7 fields (0-6) with the last one having the string "unoccupied", or else they have 9 fields (0-8). If there are more fields than that, then the code below would be inappropriate.
That version does only one "if" test on each loop iteration, economizes the removal of elements from the array (and actually makes the memory available for garbage collection, unlike "delete"), and only uses split when it really has to.my $dispmode = $options{display} * 2; $dispmode |= $options{coordinate}; # bitwise OR # $dispmode values: 0 == display:0 and coordinate:0 # 1 == display:0 and coordinate:1 # 2 == display:1 and coordinate:0 # 3 == display:1 and coordinate:1 # create a distinct print sub for each mode: # (in the second one, the regex places parens around # the last two comma-delimited fields on the line if # the last field is numeric) my @modesub = ( sub { print $_[0], "<br>\n" }, sub { ($_=$_[0]) =~ s/([^,]*,-?[\d]+)$/($1)/; print "$ +_ <br>\n"}, sub { print "<tr><td>" . join("</td><td>",split /,/, $ +_[0]) . "</td></tr>\n" }, sub { print "<tr><td>" . join("</td><td>",split /,/, $ +_[0], 8) . "</td></tr>\n" } ); for ( @data ) { # the following regex will check if this element of @data has # "unoccupied" following the 6th comma, and nothing thereafter: next if( $options{"unoccupied"} == 0 && /^(?:[^,]*,){6}unoccupied$ +/ ); $modesub[$dispmode]->( $_ ); } @data = (); # added this to get rid of the data # though maybe that doesn't really matter? print "</table>" if($dispmode & 2);
Also, I think that using @data and the field values on each row as lists (rather than always addressing specific array elements by index numbers) could save some time. (It certainly makes the coding easier.)
Still, if the proportion of "unoccupied" rows is extremely large, the process will just take that much longer when all those rows are being printed. Maybe you need to allow for paging the output (e.g. 100 rows per page, or whatever)?
(updated to fix punctuation, and to change from "while (@data)" to "for (@data)" -- I think doing "shift" on each iteration in a while loop is more work than necessary.)
Another update: I noticed your links to the code and data after I posted this, and was glad to see that I had guessed right. ;-) One extra little detail: it looks like your data file might contain blank lines, but you don't seem to be checking for that -- I don't know if this might be part of the "delay" problem, but just to be sensible, you should read from the file like this:
Final update (sorry...): I tighted up the second "modesub" so it only adds parens if the line ends with digits. Also, I deleted a couple lines of code that had been superceded by updates, rather than just commenting them out.@data = grep /\S/, <DAT>; # skip blank lines chomp @data; # remove newline chars
In reply to Re: Printing data from a file in various ways
by graff
in thread Printing data from a file in various ways
by oomwrtu
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |