Perl function: findOddCount

Question

Hello,

I am trying to write a Perl findOddCount function which finds a value in the given list that occurs an odd number of times.

#!/usr/bin/perl

sub findOddCount{
    @array1 = $_[0];
    $res=0;
    for( $i=0; $i < $#array1; $i++) {
            $res=($res ^ $array1[i])
            }
    return $res
}

@a1 = (1,1,2,2,3,3,4,4,5,5,6,7,7,7,7);
@a2 = (10,10,7,7,6,6, 2,2,3,3,4,4,5,5,6,7,7,7,7,10,10);
@a3 = (6,6,10,10,7,7,6,6, 2,2,3,3,4,4,5,5,6,7,7,7,7,10,10);
@a4 = (10,10,7,7, 2,2,3,3,4,4,5,5,7,7,7,7,10,10,6);
@a5 = (6,6);
@a6 = (1);

print "odd value in a1 is "; print findOddCount(@a1); print "\n";
print "odd value in a2 is "; print findOddCount(@a2); print "\n";
print "odd value in a3 is "; print findOddCount(@a3); print "\n";
print "odd value in a4 is "; print findOddCount(@a4); print "\n";
print "odd value in a5 is "; print findOddCount(@a5); print "\n";
print "odd value in a6 is "; print findOddCount(@a6); print "\n";

Expected Output:

odd value in a1 is 6
odd value in a2 is 6
odd value in a3 is 6
odd value in a4 is 6
odd value in a5 is 0
odd value in a6 is 1

However, my actual output showed all 0:

odd value in a1 is 0
odd value in a2 is 0
odd value in a3 is 0
odd value in a4 is 0
odd value in a5 is 0
odd value in a6 is 0

Any help would be appreciated. Thanks.

There's also a logical issue here: according to your code, if there are elements 6 and 3 with an odd number of instances, it will give an output of 9. — AntonH, Dec 04 '17 at 17:46
Added `use warnings;`, then I changed i to $i in `$res=($res ^ $array1[$i])`. There is no more warning message but the output is still 0. — Brali, Dec 04 '17 at 17:48

zdim · Answer 1 · 2017-12-06T18:10:11.737

There is a direct error with how arguments are used in the sub and a number of other deficiencies. However, even with that cleaned up the approach in the posted code doesn't do what you need.

Here is a working version first

use warnings;
use strict;
use feature 'say';

my @test_data = ( 
     [1,1,2,2,3,3,4,4,5,5,6,7,7,7,7],
     [10,10,7,7,6,6, 2,2,3,3,4,4,5,5,6,7,7,7,7,10,10],
     [6,6,10,10,7,7,6,6, 2,2,3,3,4,4,5,5,6,7,7,7,7,10,10],
     [10,10,7,7, 2,2,3,3,4,4,5,5,7,7,7,7,10,10,6],
     [6,6],
     [1] 
);

for my $ra (@test_data) 
{
    say "For: @$ra";
    if (my @odds = odd_count(@$ra)) {
        say "\tValue(s) with odd count: @odds";
    }   
    else {
        say "\tNo values show odd number of times.";
    }   
}

sub odd_count {
    my %cnt;
    ++$cnt{$_} for @_;
    return grep { $cnt{$_} % 2 } keys %cnt;
}

Comments

Always start programs with use warnings; and use strict;. It is hard to overstate how important these pragmas are. It's not pedantry; it directly helps in every line of code
Arguments are passed to functions as a flattened list of scalars. So with func(@ary) the @_ in the function has (aliases) elements of @ary. Your code takes the first element, $_[0], and all must be wrong from there
The sub above takes an array as that cuts down on code and processing. But: (1) With big arrays pass by reference, odd_count($ra) (2) Since @_ aliases arguments, if it gets changed in the sub the caller's data may get changed. See this post. If this is a problem create a local copy to work with, my @ary = @_, or my @ary = @{ $_[0] }; if you passed a reference
The $#array1 is the index of the last element in @array1, not the number of elements; so with that you'd need $i <= $#array1;, not < ...
but C-style for loop is almost never needed. Iterate over index by: for my $i (0..$#ary)
The ^ operator, which returns its operands XOR-ed bitwise, can't do what you want here since your code isn't working with groups of elements that are equal but is blindly running through all of them. Also, the code can record only one value
I use the hash to collect counts of values, what is very common, and postfix notation for for
To find the odd counts I use grep, which filters those keys whose value is odd

There is also a missing `$` in `$array1[i]` in the question – zdim Dec 04 '17 at 18:15 — zdim, Dec 04 '17 at 18:15

score 0 · Answer 2 · answered Dec 04 '17 at 17:48

Try inserting some debugging statements into your code to see if the code is doing what you think it should be doing. Also put use strict and use warnings at the top of your script (and the top of every script). Those pragmas help both beginning and experienced Perl programmers catch typos and many other not-what-I-really-wanted-to-do kind of constructions.

use strict;
use warnings;

sub findOddCount {
    my @array1 = $_[0];
    print STDERR "Length of \@array1 is ",0+@array1,", expected ",0+@_,"\n";
    my $res=0;
    print STDERR "\$#array1 is $#array1, expected $#_\n";
    for (my $i=0; $i < $#array1; $i++) {
        $res=($res ^ $array1[$i]);
        print STDERR "\$array1[$i] is $array1[$i], value for \$res is $res\n";
    }
    return $res
}

Perl function: findOddCount

2 Answers2