views:

757

answers:

5

Hey simple question probably. I have a Perl script that is counting the number of occurrences of various strings in a text file. I want to be able to check if a certain string is not yet a key in the hash. Is there a better way of doing this altogether? Here is what I am doing:

foreach $line (@lines){
    if(($line =~ m|my regex|) )
    {
        $string = $1;
        if($string is not a key in %strings) # strings is an associative array
        {
            $strings{$string} = 1; 
        }
        else
        {
            $n = ($strings{$string});
            $strings{$string} = $n +1;
        }
    }
}
+12  A: 

I believe to check if a key exists in a hash you just do

if (exists $strings{$string}) {
    ...
} else {
    ...
}
cpjolicoeur
Worked good, thanks.
+3  A: 

I guess that this code should answer your question:

use strict;
use warnings;

my @keys = qw/one two three two/;
my %hash;
for my $key (@keys)
{
    $hash{$key}++;
}

for my $key (keys %hash)
{
   print "$key: ", $hash{$key}, "\n";
}

Output:

three: 1
one: 1
two: 2

The iteration can be simplified to:

$hash{$_}++ for (@keys);

(See $_ in perlvar.) And you can even write something like this:

$hash{$_}++ or print "Found new value: $_.\n" for (@keys);

Which reports each key the first time it’s found.

zoul
Yeah, the thing is I won't know ahead of time what the keys will be.
Yes, you don't need to check for presence of the key for this purpose. You can simply say $strings{$1}++ . If the key is not there, it will be added with undef as value, which ++ will interpret as 0 for you.
Arkadiy
Sure. The point is that you can replace the whole body of your cycle (under the if) with $strings{$1}++.
zoul
(Sorry, messed up the flow by ‘editing’ my comment :)
zoul
A: 

It's been a while, but can't you just go

if(!$strings{$string}) ....
AJ
Yep that works too. Thanks!
This only works if all of the keys have values that are not false. In general, that's a bad assumption. Use exists(), which is especially designed just for this.
brian d foy
@brian de foy - Ah ha. I knew I shouldn't have answered :-)
AJ
Furthermore, your construct *creates* an entry in the hash. For the question at hand this is probably irrelevant, but for other cases it might be relevant. Using exists() also circumvents this problem and does not create an entry in the hash.
@blixor: No, it doesn't. Try perl -le 'print "ok" if !$a{hello}; print keys %a'
Hynek -Pichi- Vychodil
+5  A: 

Well, your whole code can be limited to:

foreach $line (@lines){
        $strings{$1}++ if $line =~ m|my regex|;
}

If the value is not there, ++ operator will assume it to be 0 (and then increment to 1). If it is already there - it will simply be incremented.

depesz
A: 

I would counsel against using if ($hash{$key}) since it will not do what you expect if the key exists but its value is zero or empty.

RET
Those certain circumstances are only for nested keys. For this problem, exists is the answer. Don't use exists for nested keys in one shot.
brian d foy
Downvote is still a bit harsh though - the warning is not invalidated by the simplicity of the script in this question. The more important point is the issue of using if($hash{$key}) with neither defined nor exists: the "zero but true" problem.
RET
The "zero but true" thing deserves an upvote. But what you said about autovivification is simply wrong and deserves a downvote.
innaM
The warning here is true in a way - the autovivification might happen, though not with the given example - but the proposed answer with defined() has exactly the same problem, so this is no solution at all.
ijw
Indeed - fair comment. It was too early in the morning when I wrote that answer, so I've rewritten it now I'm sufficiently caffeinated.
RET
Upvoted now. It is a fair warning now the autovivification bit has been removed.
Leonardo Herrera