Neater way of checking / populating a hash required

Discussion in 'Perl Misc' started by niall.macpherson@ntlworld.com, Apr 24, 2006.

  1. Guest

    I have some code which reads a number of key / value pairs. It needs to
    store the largest value encountered for each key in the data in a hash.

    This example code does what I want but I am sure there must be a better
    way of doing it as the if(defined ...) part looks messy to me. If I
    remove the if defined(...) check , then of course the line.

    if($value > $biggest{$key})

    gives a warning

    Use of uninitialized value in numeric gt (>) at
    C:\develop\NiallPerlScripts\clpm
    14.pl line 14, <DATA> line 1.

    the first time a new key is encountered.

    Can anyone suggest a better way of writing this ?

    --------------------------------- Start code
    ------------------------------------------------------------
    use strict;
    use warnings;
    use Data::Dumper;

    my %biggest;

    while(<DATA>)
    {
    chomp;
    my($key, $value) = split(/,/);
    if(defined ($biggest{$key}))
    {
    if($value > $biggest{$key})
    {
    $biggest{$key} = $value;
    }
    }
    else
    {
    $biggest{$key} = $value;
    }
    }
    print Dumper \%biggest;

    __DATA__
    1,123
    2,234
    3,356
    2,444
    3,666
    --------------------------------- End code
    ------------------------------------------------------------


    TIA
     
    , Apr 24, 2006
    #1
    1. Advertising

  2. Anno Siegel Guest

    bugbear <bugbear@trim_papermule.co.uk_trim> wrote in comp.lang.perl.misc:
    > wrote:
    > > I have some code which reads a number of key / value pairs. It needs to
    > > store the largest value encountered for each key in the data in a hash.
    > >
    > > This example code does what I want but I am sure there must be a better
    > > way of doing it as the if(defined ...) part looks messy to me. If I
    > > remove the if defined(...) check , then of course the line.
    > >
    > > if($value > $biggest{$key})
    > >
    > > gives a warning
    > >
    > > Use of uninitialized value in numeric gt (>) at
    > > C:\develop\NiallPerlScripts\clpm
    > > 14.pl line 14, <DATA> line 1.
    > >
    > > the first time a new key is encountered.
    > >
    > > Can anyone suggest a better way of writing this ?
    > >
    > > --------------------------------- Start code
    > > ------------------------------------------------------------
    > > use strict;
    > > use warnings;
    > > use Data::Dumper;
    > >
    > > my %biggest;
    > >
    > > while(<DATA>)
    > > {
    > > chomp;
    > > my($key, $value) = split(/,/);
    > > if(defined ($biggest{$key}))
    > > {
    > > if($value > $biggest{$key})
    > > {
    > > $biggest{$key} = $value;
    > > }
    > > }
    > > else
    > > {
    > > $biggest{$key} = $value;
    > > }
    > > }

    >
    >
    > if(!exists($biggest{$key}) || $value > $biggest{$key}) {
    > $biggest{$key} = $value;
    > }


    Existence of a key doesn't guarantee a defined value. You want

    if( !defined( $biggest{ $key}) || ... ) {

    Anno
    --
    If you want to post a followup via groups.google.com, don't use
    the broken "Reply" link at the bottom of the article. Click on
    "show options" at the top of the article, then click on the
    "Reply" at the bottom of the article headers.
     
    Anno Siegel, Apr 24, 2006
    #2
    1. Advertising

  3. Uri Guttman Guest

    >>>>> "AS" == Anno Siegel <-berlin.de> writes:

    >> if(!exists($biggest{$key}) || $value > $biggest{$key}) {
    >> $biggest{$key} = $value;
    >> }


    AS> Existence of a key doesn't guarantee a defined value. You want

    AS> if( !defined( $biggest{ $key}) || ... ) {

    assuming that these values are all > 0 you can drop one hash access with
    this:

    $biggest{$key} = $value if $value > ( $biggest{$key} || 0 ) ;

    who cares if the old value was defined/exists or not. you just want to
    see if the new value is > than the old one. the || 0 is to silence the
    warning on undef.

    uri

    --
    Uri Guttman ------ -------- http://www.stemsystems.com
    --Perl Consulting, Stem Development, Systems Architecture, Design and Coding-
    Search or Offer Perl Jobs ---------------------------- http://jobs.perl.org
     
    Uri Guttman, Apr 24, 2006
    #3
  4. <> wrote:

    > Can anyone suggest a better way of writing this ?



    > if(defined ($biggest{$key}))
    > {
    > if($value > $biggest{$key})


    if ( defined($biggest{$key}) and $value > $biggest{$key} )
    {


    --
    Tad McClellan SGML consulting
    Perl programming
    Fort Worth, Texas
     
    Tad McClellan, Apr 25, 2006
    #4
  5. Brad Baxter Guest

    wrote:
    > I have some code which reads a number of key / value pairs. It needs to
    > store the largest value encountered for each key in the data in a hash.
    >


    Since you're talking hash, I'm probably reading too much into your
    sample data. But if your keys *are* all small integers, you could
    use an array. (I'm also assuming your values are > 0).

    use warnings;
    use strict;

    my @biggest;

    while(<DATA>)
    {
    chomp;
    my( $key, $value ) = split /,/;
    $biggest[ $key ] = $value if $value > ( $biggest[ $key ] || 0 );
    }

    for my $key ( 0 .. $#biggest ) {
    $_ and print "$key: $_\n" for $biggest[ $key ];
    }

    __DATA__
    1,123
    2,234
    3,356
    2,444
    3,666

    --
    Brad
     
    Brad Baxter, Apr 25, 2006
    #5
  6. Anno Siegel Guest

    Brad Baxter <> wrote in comp.lang.perl.misc:
    > wrote:
    > > I have some code which reads a number of key / value pairs. It needs to
    > > store the largest value encountered for each key in the data in a hash.
    > >

    >
    > Since you're talking hash, I'm probably reading too much into your
    > sample data. But if your keys *are* all small integers, you could
    > use an array. (I'm also assuming your values are > 0).
    >
    > use warnings;
    > use strict;
    >
    > my @biggest;
    >
    > while(<DATA>)
    > {
    > chomp;
    > my( $key, $value ) = split /,/;
    > $biggest[ $key ] = $value if $value > ( $biggest[ $key ] || 0 );
    > }
    >
    > for my $key ( 0 .. $#biggest ) {
    > $_ and print "$key: $_\n" for $biggest[ $key ];
    > }
    >
    > __DATA__
    > 1,123
    > 2,234
    > 3,356
    > 2,444
    > 3,666


    This thread kept me wondering if we have to use an ad-hoc maximum routine
    in the presence of a pre-fabricated List::Util::max. So I'm drawing it
    in. If it kicks and screams that may be because it amounts to slurping
    the entire file before reducing the data to the maxima.

    use List::Util qw( max);

    my @biggest;
    while(<DATA>)
    {
    chomp;
    my( $key, $value ) = split /,/;
    push @{ $biggest[ $key] }, $value;
    }
    $_ = max @$_ for grep defined, @biggest;

    for my $key ( 0 .. $#biggest ) {
    defined and print "$key: $_\n" for $biggest[ $key ];
    }
    __DATA__
    1,123
    2,234
    3,356
    2,444
    3,666

    The same technique would apply if a hash %biggest were used.

    Anno
    --
    If you want to post a followup via groups.google.com, don't use
    the broken "Reply" link at the bottom of the article. Click on
    "show options" at the top of the article, then click on the
    "Reply" at the bottom of the article headers.
     
    Anno Siegel, Apr 25, 2006
    #6
  7. Ben Morrow Guest

    Quoth Uri Guttman <>:
    > >>>>> "AS" == Anno Siegel <-berlin.de> writes:

    >
    > >> if(!exists($biggest{$key}) || $value > $biggest{$key}) {
    > >> $biggest{$key} = $value;
    > >> }

    >
    > AS> Existence of a key doesn't guarantee a defined value. You want
    >
    > AS> if( !defined( $biggest{ $key}) || ... ) {
    >
    > assuming that these values are all > 0 you can drop one hash access with
    > this:
    >
    > $biggest{$key} = $value if $value > ( $biggest{$key} || 0 ) ;
    >
    > who cares if the old value was defined/exists or not. you just want to
    > see if the new value is > than the old one. the || 0 is to silence the
    > warning on undef.


    Or just no warnings 'uninitialized' in an appropriately small scope
    around it: cleaner IMHO to say what you mean :).

    Ben

    --
    "If a book is worth reading when you are six, *
    it is worth reading when you are sixty." [C.S.Lewis]
     
    Ben Morrow, Apr 26, 2006
    #7
  8. Brad Baxter Guest

    Ben Morrow wrote:
    > Or just no warnings 'uninitialized' in an appropriately small scope
    > around it: cleaner IMHO to say what you mean :).


    The code below prints:

    $VAR1 = {
    '1' => '123',
    '3' => 666,
    '2' => 444
    };

    I expected it to print:

    $VAR1 = {
    '1' => 123,
    '3' => 666,
    '2' => 444
    };

    That is, I expected the '>' operator to always give
    $value the property of having been used as a number.
    But the fact that it's compared only with undef appears
    to mean it's not given that property.

    This is perl, v5.8.7 built for sun4-solaris

    -- Brad

    use warnings;
    use strict;
    use Data::Dumper;

    my %biggest;

    while(<DATA>)
    {
    chomp;
    my( $key, $value ) = split /,/;
    for( $biggest{ $key } ) {
    no warnings 'uninitialized';
    $_ = $value if $value > $_;
    }
    }

    print Dumper \%biggest;

    __DATA__
    1,123
    2,234
    3,356
    2,444
    3,666
     
    Brad Baxter, Apr 27, 2006
    #8
  9. Ben Morrow Guest

    Quoth "Brad Baxter" <>:
    > Ben Morrow wrote:
    > > Or just no warnings 'uninitialized' in an appropriately small scope
    > > around it: cleaner IMHO to say what you mean :).

    >
    > The code below prints:


    [ code snipped; it uses > to compare an uninitialized hash elem with a
    string containing a number ]

    > $VAR1 = {
    > '1' => '123',
    > '3' => 666,
    > '2' => 444
    > };
    >
    > I expected it to print:
    >
    > $VAR1 = {
    > '1' => 123,
    > '3' => 666,
    > '2' => 444
    > };
    >
    > That is, I expected the '>' operator to always give
    > $value the property of having been used as a number.
    > But the fact that it's compared only with undef appears
    > to mean it's not given that property.


    When does this matter? As a rule, in Perl you don't need to worry about
    string/number conversions, and just trust perl to do it for you
    correctly.

    Ben

    --
    Musica Dei donum optimi, trahit homines, trahit deos. |
    Musica truces mollit animos, tristesque mentes erigit. |
    Musica vel ipsas arbores et horridas movet feras. |
     
    Ben Morrow, Apr 27, 2006
    #9
  10. Brad Baxter Guest

    Ben Morrow wrote:
    > Quoth "Brad Baxter" <>:
    > > Ben Morrow wrote:
    > > > Or just no warnings 'uninitialized' in an appropriately small scope
    > > > around it: cleaner IMHO to say what you mean :).

    > >
    > > The code below prints:

    >
    > [ code snipped; it uses > to compare an uninitialized hash elem with a
    > string containing a number ]
    >
    > > $VAR1 = {
    > > '1' => '123',
    > > '3' => 666,
    > > '2' => 444
    > > };
    > >
    > > I expected it to print:
    > >
    > > $VAR1 = {
    > > '1' => 123,
    > > '3' => 666,
    > > '2' => 444
    > > };
    > >
    > > That is, I expected the '>' operator to always give
    > > $value the property of having been used as a number.
    > > But the fact that it's compared only with undef appears
    > > to mean it's not given that property.

    >
    > When does this matter? As a rule, in Perl you don't need to worry about
    > string/number conversions, and just trust perl to do it for you
    > correctly.


    I can't give a good example of when it might matter. Comparing
    Dumper output in tests, maybe? (Not saying that's a good example.)
    Mainly, I'm asking for my own edification and on the off chance that
    this behavior might be a symptom of a subtle bug.

    Apparently, when an operand is a string containing a number, the
    only time it is not affected this way by the '>' operator (or '<',
    '==', etc.) is when the right hand side is undef. That just doesn't
    seem right. Does it matter? I can't say for sure, but it seems
    inconsistent.

    # use warnings;
    use strict;
    use Data::Dumper;
    $Data::Dumper::Terse++;
    $Data::Dumper::Indent=0;

    my $x = '1'; my $y;
    show();

    $y = '0';
    show();

    $x = '1'; $y = 'a';
    show();

    $x = undef; $y = '1';
    show();

    sub show {
    my $dummy = $x > $y;
    print Dumper [$x, $y];
    }

    __END__
    ['1',undef][1,0][1,'a'][undef,1]

    --
    Brad
     
    Brad Baxter, May 1, 2006
    #10
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Stimp
    Replies:
    5
    Views:
    481
    Stimp
    Oct 14, 2005
  2. Ross Clement (Email address invalid - do not use)

    Neater method of creating a linked list

    Ross Clement (Email address invalid - do not use), Nov 30, 2005, in forum: C Programming
    Replies:
    2
    Views:
    282
    Richard Harnden
    Nov 30, 2005
  3. Bill Cunningham

    neater code

    Bill Cunningham, Jan 31, 2010, in forum: C Programming
    Replies:
    14
    Views:
    648
    Ian Collins
    Mar 1, 2010
  4. rp
    Replies:
    1
    Views:
    555
    red floyd
    Nov 10, 2011
  5. Srijayanth Sridhar
    Replies:
    19
    Views:
    639
    David A. Black
    Jul 2, 2008
Loading...

Share This Page