Brian Ling wrote: > Hi list, > > The following code fragment works but has a horrible feel about it, > can anyone suggest ways to improve it. > > The data structure is generated by an XML parser. > > for ( keys %{$parsed_data->{value}->{NetworkServices}->{value}} ) { > if ( > $parsed_data->{value}->{NetworkServices}->{value}->{$_}->{value}->{Inter > face}->{value}->{DeviceName}->{value} eq "modem" and > > $parsed_data->{value}->{NetworkServices}->{value}->{$_}->{value}->{PPP}- >> {value}->{UserDefinedName}->{value} eq $serviceName) { > print "found correct modem\n"; > > $parsed_data->{value}->{NetworkServices}->{value}->{$_}->{value}->{PPP}- >> {value}->{AuthName}->{value} = $username; > } > }
Hi Brian. As Jenda said, use temporary variables. Also you can go through all of the _values_ in the hash instead of all the keys, which is all you need here since you're only using the key to index the hash. Thirdly any }->{ or ]->[ pair can be abbreviated to }{ or ][. So how about this: for ( values %{$parsed_data->{value}{NetworkServices}{value}} ) { my $iface = $_->{Interface}{value}; next unless $iface->{DeviceName}{value} eq "modem"; my $ppp = $_->{PPP}{value}; next unless $ppp->{UserDefinedName}{value} eq $serviceName; print "found correct modem\n"; $ppp->{AuthName}{value} = $username; } HTH, Rob -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]