Hi Stuart, a few comments on your code.
On Wednesday 01 September 2010 21:18:10 Kryten wrote: > Wow. Thank you Shlomi, Thank you Chas and Thank you Shawn. > > Hash sets seem to be the way to go here. Much quickness too! > > Here is what I have ( the least I can do is give you all a chance to > laugh > at my code! ):- > > > #!/usr/bin/perl > use warnings ; Add "use strict;" too. See: http://perl-begin.org/tutorials/bad-elements/ > > my $names_file = 'C:\names.log' ; Don't call variables "file". See: http://perl-begin.org/tutorials/bad-elements/ > my $exclude_list = 'C:\exclude.txt' ; It's probably safer to use 'C:/exclude.txt' because the backslash has special meaning in perls. > my %names_hash ; No need to call a hash variable with a name trailing with "_hash". > > open ( NAMES, $names_file ) ; > open ( EXCLUDE, $exclude_list ) ; See: http://perl-begin.org/tutorials/bad-elements/#open-function-style > > @allnames = <NAMES> ; > @allexcludes = <EXCLUDE> ; Make sure you declare all variables using "my". Also see: http://perl-begin.org/tutorials/bad-elements/#identifiers-without-underscores > > #---> Create the hash set #---> > foreach $exclusion ( @allexcludes ) { > chomp( $exclusion ) ; > $names_hash{ $exclusion } = 1 > } This would be done faster, by saying: chomp(@allexcludes); my %names_hash = (map { $_ => 1 } @allexcludes). > > #---> Walk the names array #---> > foreach $name ( @allnames ) { In this case it would consume less memory not to slurp a file needlessly and use: http://perl-begin.org/tutorials/bad-elements/#foreach-lines > chomp( $name ) ; > if ( ($dn) = $name =~ /(\d{5,6})/ ) { > if ( $names_hash{ $dn } ) { > print "This would be excluded: $name\n" > } # Close inner IF > } # Close outer IF Why do you have comments such as "#Close IF"? Everything can see this is the case by following the indentation and by using their editor's "go-to-matching brace" feature (e.g: the "%" key in http://www.vim.org/ ). Regards, Shlomi Fish > } > > This seems to work; I'm aghast at how quickly it tears through the > data. > My Powershell script was taking 2-3 minutes. This method completes in > about 4 seconds. Though to be fair I'm not using an associative array > in > my Powershell code, so it's not this same. > > This is an order of magnitude better than what I am doing now, so big, > big > thanks and I'd be delighted for any pointers for how to make my code > more .. perl'ish. > > Thanks, > Stuart -- ----------------------------------------------------------------- Shlomi Fish http://www.shlomifish.org/ Interview with Ben Collins-Sussman - http://shlom.in/sussman God considered inflicting XSLT as the tenth plague of Egypt, but then decided against it because he thought it would be too evil. Please reply to list if it's a mailing list post - http://shlom.in/reply . -- To unsubscribe, e-mail: beginners-unsubscr...@perl.org For additional commands, e-mail: beginners-h...@perl.org http://learn.perl.org/
