in reply to Concordance Generator
my @theseWords = undef; is nonesense. Arrays are not undef. Only scalars are undef. It's being converted to (), which is what my @theseWords; by itself would do anyway.
Now for the hash you are using (): my %Count = (); again, that's unnecessary since it's created empty in the first place.
Now look at my ( $line, $word, $count, $LineNum ) = undef; what do you think that does? Since the undef is converted to an empty list (someone correct me if I'm wrong, please), then you have a list of 4 lvalues being assigned an empty list. That is treated like (undef, undef, undef, undef) which again is what my would have done anyway.
More subtle, little-known issue: The $VERSION should be a number of a v-string, not a string. See Everything you wanted to know about Module Version Numbers and Checking for the issues and compatability things.
open ( IN, $file ); ... or die;?
Re: $word = $opt_s; Word(); That is nasty!. The variable $word is being used for loop control in a couple places, and the same variable is used for passing a parameter to this function. Pass a parameter instead of using a global! The only occurance of $word other than the ones which are loop control (set within the loop) is one use within Word(). Say: Word($opt_s) if $opt_s; and use the parameter in Word with my $word= shift;.
Meanwhile take $word out of the globals at the top and add it to each foreach loop instead. With strict turned on, you will find these.
Why zero out the array when you assign to it in the very next line? And again, you're using the same variable for different purposes throughout the code: it's essentially local to this outer foreach loop, and then it's used again for the print loop later. Use my to make it more local to the purpose, whether or not you use the same name.undef @theseWords; @theseWords = split( / /, $line );
s/\s\s*?//g I'll let japhy tell you about that one. I think you want \s+?. But, if you split on whitespace (see docs on split) you don't need to do that at all! Extra blanks are no trouble.
So you are keeping a list of found lines as a long string, and not adding the same number twice. Why not use a list instead of a long string, so you don't have to parse it out each time? You won't have to duplicate the assignment as a special case of being the first time, either:$Line{$word} =~ m/(\d*?)$/; if ( $1 == $count ) { next; } else { $Line{$word} .= ", $count";
Re sort { $a cmp $b }, that's what sort does normally. And it will run a lot faster if you don't use a sort block. So, leave off the useless comparison block.push @{$Line{$word}}, $count unless exists $Line{$word} && $Line{$word}[-1] == $count;
Keep on coding...
—John
</code>
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Re: Concordance Generator
by sifukurt (Hermit) on Aug 15, 2001 at 01:33 UTC | |
by John M. Dlugosz (Monsignor) on Aug 15, 2001 at 01:48 UTC |