in reply to Pushing anon hash on to HoA structure giving Use of uninitialized value in hash element error using warnings

Can anybody shed any light on this?

Not directly, but there are a couple of issues that leap out at me.

if ($token->[0] eq "S" and ${$token->[2]}{year}){ $year = ${$token-> +[2]}{year} } if ($token->[0] eq "S"){

Those two statements are actually at the same level. Despite what the indentation says, the latter is not subordinate to the former. Bring the second back to the same indentation level as the first, or, better yet, rearrange the code to say $token->[0] eq "S" only once.

The variable $temp raises an immediate Red Flag. Name it $token instead, because that is what it appears to be.

Declare your variables as late as possible. By pulling $temp/$token into the inner loop, and above all, $info, you don't need the make-work $info = {}, it will be automatically reset each time through the loop.

Drop the & on the function calls.

The following code:

$temp = &get_token("a"); if ($temp eq " "){ $info->{email} = ''; } else{ $info->{email} = $temp; }

... could greatly benefit from the ternary ? : operator:

$token = get_token("a"); $info->{email} = $token eq " " ? '' : $token ; # update: had a mish-mash of temp/token in this snippet

... not because using the ternary operator is cool, or for some other bogus style reason, but because it means that you only say $info->{email} once, which means one less chance of typing or editing a key incorrectly and thereby creating two keys in the hash.

The following code, um, needs attention:

my $t2; ($info->{city}, $t2) = split(', ',$temp); ($info->{state},$t2) = split(" ",$t2); $t2 =~ s/\s+//; $info->{zip} = $t2;

It looks like an abuse of split (if you have is a hammer...) A regex can do the same in a single statement, with no need for the intermediate variable (which has a horribly unhelpful name anyway). I'm not sure about the following, but something like this will work:

@{$info}{qw{ city state zip}} = ($token =~ /^([^,])+,\s+(\S+)\s+(\d+)$/) ? ( $1, $2, $3 ) : ( '', '', '' ) ;

Now that's definitely fancy stuff, but it's just taking the previous points into consideration and throwing in a hash-slice assignment. And with a bit of creative whitespace we can make things line up and the code documents itself quite nicely.

- another intruder with the mooring of the heat of the Perl

  • Comment on Re: Pushing anon hash on to HoA structure giving Use of uninitialized value in hash element error using warnings (style issues)
  • Select or Download Code

Replies are listed 'Best First'.
Re^2: Pushing anon hash on to HoA structure giving Use of uninitialized value in hash element error using warnings (style issues)
by Popcorn Dave (Abbot) on Jul 05, 2004 at 17:32 UTC
    Thanks for the tips on the ternary. I had tried that but obviously I had done something wrong and went with the if-else instead. I prefer the ternary as well when I can as I think it's easier to read.

    I see your point as far as $token->[0] eq "S". I'm trying to see if the S token has an id="year" in it. If it didn't have the id="year" in it, I wanted it to drop through, but I see now how I can rewrite that.

    As far as the abuse of split, yes that was going to get reworked. :) This is really the first time I've dealt with a complex data structure so that was my primary objective.

    There is no emoticon for what I'm feeling now.