> 
> ----- Original Message ----- 
> From: "Wiggins d Anconia" <[EMAIL PROTECTED]>
> To: "Mark Goland" <[EMAIL PROTECTED]>; <[EMAIL PROTECTED]>
> Sent: Thursday, September 16, 2004 1:27 PM
> Subject: Re: HoH not building as expected
> 
> 
> > <snip>
> >
> > > > >
> > > > > I expact the output to be
> > > > >
> > > > > Unknown =>:
> > > > > VENDOR :Unknown
> > > > > CPU-MODEL :model1
> > > > > Unknown =>:
> > > > > VENDOR :Unknown
> > > > > CPU-MODEL :Unknown
> > > > >
> >
> > I am confused as to why you think you need a HoHoH, when what you
> > originally displayed above is really a HoAoH because the first 'Unknown'
> > has multiple entries, which is impossible to replicate with a Hash since
> > it is a one-to-one relationship, one key to one value.  If you still
> > feel so and are having problems, provide us more of the data and a
> > larger example of what you expect and we will be able to get it sorted.
> >  Fast access through a hash is good, but correct access even if
> > requiring a loop is always better than the wrong structure for the data.
> 
> Hello again my friends,
>     Basicly what I'll have a large set of input data that I need to match
> against a static list.
> 
> I want to have following structure
> Vendor Model Atributes
> 

This doesn't seem to match at all the code you provided, either the data
you are showing us (which still isn't really enough for us to tell since
you have given us data with lots of "Unknown" etc. in it.  Give us real
data.

> So once I have my HoHoH I can say something like
> if exists $RATING{Vendor}{Type}{Quntity} and $RATING{Vendor}{Type}{Color}
> 

I think I have it now, but the biggest problem with the code is that you
are using the wrong array indices and you have data on a different line
than you are "currently interpreting.  Your spacing and comments also
make it very hard (at least for me, I know this is a style issue) to
read what you are actually doing....

> Here is an example with more data...
> 
> #!PERL
> use warnings;
> use strict;
> 
> my (@te,%RATING,$debug,$attrib,$key,$CPU_ATTR);

Again, declaring all of these here defeats the purpose of strictures,
and is probably one of the reasons why your code is so confusing.

> $debug=0;
> 
> @te = (   "CREATE   CPU-MODEL UNKNOWN_CPU VENDOR Unknown, GENERIC_CPU
VENDOR
> SUN\n" ,
>   "Quntity 5\n" ,
>   "Color Blue\n" ,
> 
>    "CREATE   CPU-MODEL model1 VENDOR Unknown, GENERIC_CPU VENDOR
Unknown\n",
>   "Quntity 6\n" ,
>   "Color Blue\n" ,
>  );
>

Both of the above declarations can have the C<my> moved onto them, it is
just a good habit to get into.
 
> s/,|^\t//g for @te;
> 
> 
> foreach ( @te ){
> my $input = $_;
> 

If you are going to name the variable you can do it all in one shot, the
above can be written:

foreach my $input (@te) {

> my @fields= split /\s+/,$input;
> print "LINE=$input" if $debug;
> 
> 
> if( $fields[0] eq 'CREATE' ){
>         $CPU_ATTR= {};
>         $RATING{$fields[5]} = $CPU_ATTR;#

What is the extra # mark for? Index 5 maps to 'GENERIC_CPU' in your
data, should this be 4?  You also need to build your 'type' into the
hash structure at this point.

>         $CPU_ATTR->{$fields[2]}=$fields[3]; #processor [EMAIL PROTECTED]
>         $CPU_ATTR->{$fields[4]}=$fields[5]; #processor [EMAIL PROTECTED]
> 

I have no idea what the above are doing, I think this is the problem
with the structure, and would be at least a little bit indicated if you
used better scoping rules.

> }
> if( $fields[0] eq 'Quntity' ){
>  $CPU_ATTR->{$fields[0]}=$fields[1];
> }
> if( $fields[0] eq 'Color' ){
>  $CPU_ATTR->{$fields[0]}=$fields[1];
> }

The problem is that $CPU_ATTR really shouldn't still be alive here,
because you have transitioned to a new line, so the problem is that at
this point you can't index by type, which means your structure is one
level flatter than it should be.

> 
> }# end of while
> 

Wrong comments are much worse than superfluous comments which are much
worse than no comments. Personally I can't stand comments telling me
that i have ended a loop, it should be obviuos (and that is what code
folding is for), but that is a style issue.  But it isn't the end of the
'while' it is the end of a 'foreach', *especially* since the next
statement is a 'while'.

> while ( my ($key, $value) = each(%RATING) ) {
>         print "$key =>:\n";
>         for $attrib ( keys %$value ){
>                 print "$attrib :";print $value->{$attrib} . "\n";
>         }
> print "\n\n";
> }
> 

This will need to be improved once the structure is corrected, but I
will leave that to you, again Data::Dumper is your friend.

Assuming I understand the spec correctly, this is what I would probably
produce:

-- UNTESTED --

#!/usr/local/bin/perl
use strict;
use warnings;

my $debug=0;

my @te = ( "CREATE   CPU-MODEL UNKNOWN_CPU VENDOR Unknown, GENERIC_CPU
VENDOR SUN\n" ,
           "Quntity 5\n" ,
           "Color Blue\n" ,

           "CREATE   CPU-MODEL model1 VENDOR Unknown, GENERIC_CPU VENDOR
Unknown\n",
           "Quntity 6\n" ,
           "Color Blue\n" ,
         );

s/,|^\t//g for @te;

my $current;
my $rating;
foreach my $input (@te) {
    print "LINE=$input" if $debug;

    my @fields= split /\s+/, $input;

    if ($fields[0] eq 'CREATE') {
        $rating->{$fields[4]}->{$fields[2]} = $current = {};
    }
    if ($fields[0] eq 'Quntity') {
        $current->{$fields[0]} = $fields[1];
    }
    if ($fields[0] eq 'Color') {
        $current->{$fields[0]} = $fields[1];
    }
}

use Data::Dumper;
print Dumper($rating);

Though the use of C<$current> could be avoided if you could read
multiple lines at a time, if that is possible I would switch to that
method. If not the above should do.

If you are confused or this doesn't work, give it another shot and
provide at least 10 entries of data.

http://danconia.org

-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>


Reply via email to