http://qs321.pair.com?node_id=411019

Paulster2 has asked for the wisdom of the Perl Monks concerning the following question:

I want to be able to dynamically update my hash. I am currently using the following to initialize it: 

@Names = split(/\|/, &getIAMparameter(IAMNames));
@Ip = split(/\|/, &getIAMparameter(IAMIp));

%ServerStatus = (  "$Ip[$i]"    =>  ["$Names[$i]",     "unknown"],
                   "$Ip[$i+1]"  =>  ["$Names[$i+1]",   "unknown"],
                   "$Ip[$i+2]"  =>  ["$Names[$i+1]",   "unknown"],
                   "$Ip[$i+3]"  =>  ["$Names[$i+1]",   "unknown"],
                   "$Ip[$i+4]"  =>  ["$Names[$i+1]",   "unknown"] );

[download]
The first two lines of the code goes out and extracts data from a config file, that being server names and IP addresses. With the code shown I have to update it if the number of entries changes in the configuration file. I want to replace it with something like this:
@Names = split(/\|/, &getIAMparameter(IAMNames));
@Ip = split(/\|/, &getIAMparameter(IAMIp));
$num = $#Ip;

foreach ($i=0;$i<=$num;$i++) {
    %ServerStatus = ( "$Ip[$i]" => ["$Names[$i]",   "unknown"] );
}

[download]
But only get one entry where there should be five. The $num = $#Ip; seems to work just fine, at least it gives me the number back that I am expecting, so I take it that there is something wrong with my foreach loop. I am by no means an expert at hashes (yes, I know, some consider it the life blood of perl!!). I was wondering if someone had a better mouse trap for me.

Many thanks in advance.

Paulster2


You're so sly, but so am I. - Quote from the movie Manhunter.

UPDATE: Thanks for the typo correction, Joost, I have to retype stuff over from another system. That part of the code is typed correctly on that system. I just fat fingered it here. Also, as I stated previously, I have inherited this code from another perl monger who did not use strict. With a couple thousand lines of code, it would make it a little difficult to implement strict now. I try to almost strictly use strict (pun intended). Maybe I will take the time to go back and do that, it may be worth the time, even if for only a learning experience! Thanks again to all for your responses!

Replies are listed 'Best First'.
Re: Initializing a hash using a foreach loop
by dragonchild (Archbishop) on Nov 29, 2004 at 19:34 UTC 
    %ServerStuatus = ( "$Ip[$i]" => ["$Names[$i]",   "unknown"] );

    [download]

    That line reinitializes the hash every time. 

    $ServerStuatus{"$Ip[$i]"} = ["$Names[$i]",   "unknown"];

    [download]
    should do what you want.

    Being right, does not endow the right to be rude; politeness costs nothing.
    Being unknowing, is not the same as being stupid.
    Expressing a contrary opinion, whether to the individual or the group, is more often a sign of deeper thought than of cantankerous belligerence.
    Do not mistake your goals as the only goals; your opinion as the only opinion; your confidence as correctness. Saying you know better is not the same as explaining you know better.

[reply]
[d/l]
[select]
Re: Initializing a hash using a foreach loop
by Roy Johnson (Monsignor) on Nov 29, 2004 at 19:37 UTC
    You're re-assigning the whole hash every time. Try: 
    %ServerStatus = ();
foreach my $i (0..$num) {
  $ServerStatus{$Ip[$i]} = [$Names[$i], 'unknown'];
}

    [download]
    I've also removed unnecessary quotes and converted to a more Perlesque foreach loop. An alternative method would be to use map to assign the whole hash: 
    %ServerStatus = map {($Ip[$_], [$Names[$_], 'unknown'])} 0..$num;

    [download]
    Caution: Contents may have been coded under pressure.
[reply]
[d/l]
[select]
Re: Initializing a hash using a foreach loop
by Zaxo (Archbishop) on Nov 29, 2004 at 19:40 UTC

    That is faster and tidier with a hash slice, 

    my %ServerStatus;
@ServerStatus{split( /\|/, getIAMparameter('IAMIp'))} = 
    map {[$_,'unknown']} split /\|/, gwtIAMparameter('IAMNames');

    [download]
    Keeping IAMNames and IAMIp synchronized in your configuration may be a maintainance problem for you.

    After Compline,
    Zaxo

[reply]
[d/l]
Re: Initializing a hash using a foreach loop
by BrowserUk (Patriarch) on Nov 29, 2004 at 19:42 UTC

    Replace this 

    foreach ($i=0;$i<=$num;$i++) {
    %ServerStuatus = ( "$Ip[$i]" => ["$Names[$i]",   "unknown"] );
}

    [download]

    With 

    %ServerStatus = map{ 
    $Ip[$_] => [ $Names[$_], "unknown" ] 
} 0 .. $#Ip;

    [download]

    Please note that quoting all your variables that way is

    1. Unnecessary.
    2. Is making you code harder to read than it need be, and requiring you to type more.
    3. Making Perl work harder than it needs to.
    4. Under some circumstances, can cause errors. Specifically, when passing variables to built-ins that expect pass-by-reference.

    Also, you've misspelt %ServerStuatus. Not that I can say anything, my spelling is atrocious. But I always use strict, so if I later spell it correctly, Perl would warn me. Without it, as your code appears to be, it won't be able to.

    Examine what is said, not who speaks.
    "But you should never overestimate the ingenuity of the sceptics to come up with a counter-argument." -Myles Allen
    "Think for yourself!" - Abigail        "Time is a poor substitute for thought"--theorbtwo         "Efficiency is intelligent laziness." -David Dunham
    "Memory, processor, disk in that order on the hardware side. Algorithm, algorithm, algorithm on the code side." - tachyon
[reply]
[d/l]
[select]
Re: Initializing a hash using a foreach loop
by Joost (Canon) on Nov 29, 2004 at 19:43 UTC
[reply]
[d/l]
Re: Initializing a hash using a foreach loop
by gaal (Parson) on Nov 29, 2004 at 19:40 UTC
    You are overwriting %ServerStatus in each iteration of your loop. Instead of your assignment, try:

    $ServerStatus{ $Ip[$i] } = [ $Names[$i], "unknown" ];

    You can also use map and one assignment instead of foreach:

    %ServerStatus = map { $Ip[$i] => [ $Names[$i],   "unknown"] } 0 .. $#Ip;

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom