in reply to better way to printing random hash key and its value

Well, there are a few places that it could be more efficient.

The very first if in your foreach loop tests for a condition ($netHash{$val} > 1) that's guaranteed by your input loop.

Inside your conditions, you check that the value from $netHash is defined before assignment, but that's also guaranteed by your input loop.

That list of if conditions in the body of foreach all look mutually exclusive, and you're essentially doing "and" with nested conditions. You could rewrite this way:

# you can get rid of the ($netHash{$val} > 1.0) test # I only included it to make the example more like yours if($netHash{$val} > 1.0 && $netHash{$val} < 1.4) { my ($x1, $y1) = ($val,$netHash{$val}); $capVal_1_Hash{$x1} = $y1; } elsif($netHash{$val} > 1.4 && $netHash{$val} < 1.9) { my ($x2, $y2) = ($val,$netHash{$val}); $capVal_2_Hash{$x2} = $y2; } ... ...

That way, you're not checking that a value is between 1.4 and 1.9 after you've already established that it's between 1.0 and 1.4.

I notice you seem to be excluding some values (such as 1.4), and I wonder if that's intentional.

Your printHash might be faster if you don't call keys twice. Like so:

sub printHash { my $href = shift; my @href_keys = keys %{$href}; my $RandKey = $href_keys[int(rand(scalar @href_keys))]; print "$RandKey $href->{$RandKey}\n"; }

You seem to have a hundred separate hashes, %capVal_xx_Hash. I'd make those one hash of hashes, but I don't think that will gain you any speed. In that case your $capVal_100_Hash{$x100} = $y100 would be $capVal_Hash{100}{$x100} = $y100, just as an example. Then your repeated calls to printHash at the end could be in a loop:

foreach my $n ( 1 .. 100 ) { printHash( $capVal_hash{$n} ); }

In that case, you could even factor out the sub printHash and just put it in the loop body, if you wanted. That might be faster (it saves a function call).

If you want to make the code shorter still (y'know, if you're not getting paid by the line), you could put your conditions in an array to loop over. (This is instead of the if ... elsif construct I suggested above.)

my @range_destination = ( [ 1.0, 1.4, 1 ], [ 1.4, 1.9, 2 ], ... [ 99.2, 99.8, 99 ], [100.1, 100.9, 100 ], ); # what you call $val is really a KEY, # but I'll keep your name anyway. while ( my ( $val, $real_val ) = each %netHash ) { CONDITION: foreach my $cond_ref ( @range_destination ) { my ( $min, $max, $dest ) = @$cond_ref; if ( $val > $min && $val < $max ) { $capVal_Hash{$dest}{$val} = $real_val; last CONDITION; } } }

Using each is more memory efficient. Putting all your ranges in one places makes it easier to see what they are (though I can't tell if that's significant), and it looks like it might be a few hundred lines shorter.

Anyway, hope these suggestions help.

Replies are listed 'Best First'.
Also, why two loops?
by kyle (Abbot) on Dec 23, 2006 at 05:51 UTC

    You could...

    LINE: while(<IFH>) { chomp; my($nx,$ny) = split; next LINE if ($ny <= 1.0); CONDITION: foreach my $cond_ref ( @range_destination ) { my ( $min, $max, $dest ) = @$cond_ref; if ( $nx > $min && $nx < $max ) { $capVal_Hash{$dest}{$nx} = $ny; last CONDITION; } } }

    One loop instead of two will save some time, I reckon.

    If you know the frequency of values in your input, you can also order the @range_destination list so that the more frequent conditions are first, short circuiting the CONDITION loop faster.