perlnewby:

I'd suggest using strict and warnings. That would point you to one of your problems.

Learning the basic syntax would also be helpful. Nearly every line has some sort of syntax error or unusual construct that doesn't do what you think it means.

You should also show how you're calling your subroutine. I can see a couple different ways to do it, and the fixes I'd suggest would depend on which one you're wanting.

Finally, your subroutine isn't actually returning anything. So if you want it to return the mode, you'll need to tell it to do so.

Since you're just learning, I'd suggest you also use Data::Dump, and rather than build your code all at once and then wonder what's wrong, build it a step or two at a time, using Data::Dump to display the values you have so you can figure out what the next step would be.

So I'd start with this:

use strict;
use warnings;
use Data::Dump 'pp';

sub mode_num {
   my %count = ();

   my %opt = %{ shift @_ };
   print pp(\%opt),"\n";
}

my @numbers = (
    2, 3, 3, 3, 5, 7, 8, 12, 32, 44, 55, 12, 3,
    23, 43, 33, 1, 4, 25, 43, 42, 1, 4, 5, 3, 3, 3
);

my $mode = mode_num(@numbers);
print "Mode is: $mode\n";
[download]

With this, when you run it, you'll get the message:

Can't use string ("2") as a HASH ref while "strict refs" in use at xyz
+.pl line 8.
[download]

So that means you've got a problem in line 8. So split line 8 into two operations, perhaps something like this:

    my $temp = shift @_;
    print "temp is ", pp($temp>), "\n";
    my %opt = %{ $temp };
[download]

When you run it *this* way, you'll get:

temp is 2
Can't use string ("2") as a HASH ref while "strict refs" in use at xyz
+.pl line 10.
[download]

So does $temp hold what you're expecting? if so, you need to figure out what you're doing wrong on line 10. Otherwise, figure out what you're doing to get the value of $temp. Once you get $temp correct, and then correctly build %opt from it, you can merge the two steps back together. Then, add the next step and get it working the way you want. Without too much effort, you should be able to, step by step, get the mode.

Beginners frequently try to build large chunks of code and then get lost trying to figure out how to fix it. Until you gain enough experience, it's better to build it a little at a time, testing it each step of the way. That will help you be successful faster, as well as teaching yourself several interesting bits about perl during each step.

...roboticus

When your only tool is a hammer, all problems look like your thumb.

There are far better and shorter ways than this one, but I'm hoping this will allow you to see what's happening. If I understand you correctly, you want to extract the number in the input that has the highest 'count'. If so:

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

my @data = qw(2 3 3 3 5 7 8 12 32 44 55 12 3 23 43 33 1 4 25 43 42 1 4
+ 5 3 3 3);
my %seen;

for my $num (@data){
    $seen{$num}++;
}

my @keys = sort {$seen{$b} <=> $seen{$a}} keys %seen;

my $highest = $keys[0];
my $count = $seen{$keys[0]};

print "Highest count: $highest, Count: $count\n";
[download]

It defines an idiomatic %seen hash for keeping track, and using each value in the data, sets the key as the number in data, and adds one to the current value. Then, we create a keys array with a backwards sort which populates the array with the data number that has the highest value (count) to lowest. We then set the count value to $count and the key (data number) to $highest.

If I misunderstood what you need, just clarify.

-stevieb

This assumes unimodal datasets:

sub mode{ 
    my %h; ++$h{ $_ } for @_; 
    return ( sort{ $h{ $b } <=> $h{ $a } } keys %h )[ 0 ] 
};;

@data = qw[2 3 3 3 5 7 8 12 32 44 55 12 3 23 43 33 1 4 25 43 42 1 4 5 
+3 3 3];;

print mode( @data );;
3
[download]

You'd need to so a bit more work to discover bi/multimodal datasets.

I didn't try to run your code since it's not a complete working program. Here's one way to get what I think you want:

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

my %hash;

%hash = map { $_ => ++$hash{ $_ } } (split ' ', <DATA>);

for (sort { $hash{$b} <=> $hash{$a} } keys %hash) {
  print "$_ => $hash{ $_ }\n"; 
  last;
}
__DATA__
2 3 3 3 5 7 8 12 32 44 55 12 3 23 43 33 1 4 25 43 42 1 4 5 3 3 3
[download]

Yes, I have already found a way to get mode for the set of numbers in a couple of ways.

my ($mode_num)=alt_mode_num(@data);
print "Mode : $mode_num\n";

 sub alt_mode_num{
        my %count=();
  
    for (sort{$a <=> $b} @_){
        $count{$_}++;
        }
   my @key= sort{$count{$b}<=>$count{$a}} keys %count;
   #print " Key with highest value is $keys[0] \n";
   return $key[0];       
       
       }
[download]

BUT I saw this way of passing the values by ref in this forum and wanted to duplicated it but failed

this is the model I used to try to get my mode sub to look like

my @num =(1..10);
my @words = qw(hello world inside);
function( {NUMBERS=>\@num, WORDS=>\@words} );
exit;
sub function {
  die "no parameter!\n" unless @_;
  my %opt = %{ shift @_ };

  my @i_words = $opt{WORDS};
  print "this r words", Dumper \@i_words;

  my @i_numbers = $opt{NUMBERS};
  print Dumper \@i_numbers;
}
[download]

(one of my problemS) is getting a counter and a compare for highest value.

I specifically wanted this so I can see how it is done properly when using this way

Yes, I have already found a way to get mode for the set of numbers in a couple of ways.

No need to sort the list before you populate the hash.

(one of my problemS) is getting a counter and a compare for highest value.

Well, yes, you'll still have to count the values per key, so you'll still need a hash in any reasonable solution. The clever stuff with a reference to the array only gets your data into your subroutine. The next steps: to sort it in order to find out the mode number, and to return the mode, are going to be the same however you get the data into the sub.

But you still aren't using strict and warnings and using a dumper to help you develop your code, as roboticus suggested in one of the most generous posts I can remember. If you were, you would get as far as:

#!/usr/bin/perl -w
use strict;
use Data::Dumper;

my @data = ( qw/ 2 3 3 3 5 7 8 12 32 44 55 12 3 23 43 33 1 4 25 43 42 
+1 4 5 3 3 3 /);

my $mode = mode( { NUMBERS => \@data } );

sub mode {
    my %opt = %{ shift @_ };
    print Dumper \%opt;
}

__END__
[download]

. . . and you would see the contents of %opt:

$VAR1 = {
          'NUMBERS' => [
                         '2',
                         '3',
                         '3',
                         '3',
                         '5',
                         '7',
                         '8',
                         '12',
                         '32',
                         '44',
                         '55',
                         '12',
                         '3',
                         '23',
                         '43',
                         '33',
                         '1',
                         '4',
                         '25',
                         '43',
                         '42',
                         '1',
                         '4',
                         '5',
                         '3',
                         '3',
                         '3'
                       ]
        };
[download]

It could be that viewing the data like that doesn't help you see what's going on (because of the complicated way of passing the argument to the sub) ... so you can try a simpler way:

#!/usr/bin/perl -w
use strict;
use Data::Dumper;

my @data = ( qw/ 2 3 3 3 5 7 8 12 32 44 55 12 3 23 43 33 1 4 25 43 42 
+1 4 5 3 3 3 /);

my $mode = mode( { NUMBERS => \@data } );

sub mode {
      my %opt = %{ shift @_ };
      while (my ($key, $val) = each %opt) {
            print "k:$key v:$val\n";
      }   
}

__END__
[download]

This outputs:

k:NUMBERS v:ARRAY(0x7fef9a82a168)
[download]

So now you can see that your %opts hash has only one key=value pair, and your list of numbers is still encapsulated in a reference in the single value in the hash. Probably not what you want, since your next line,

my @i_numbers = $opt{NUMBERS};
[download]

is expecting the value of the hash element to be an array. It's not. It's a reference to an array. So in order to use it, you'd either have to dereference it as you extract it from the hash, or capture into a reference:

my $i_numbers = $opt{NUMBERS}; # $i_numbers is an arrayref

# or

my @i_numbers = @{ $opt{NUMBERS} }; # dereference the arrayref as you 
+get it from the hash
[download]

. . . but either way, you are right back where you would have been if you had just passed a simple array to the sub: you still have to build a lookup table, i.e. a hash, in order to be able to sort by how many occurrences of each number there are. Here's the complete program: you can see that it has a level of complexity that is not needed, because of referencing the array at the beginning:

#!/usr/bin/perl -w
use strict;
use Data::Dumper;

my @data = ( qw/ 2 3 3 3 5 7 8 12 32 44 55 12 3 23 43 33 1 4 25 43 42 
+1 4 5 3 3 3 /);

my $mode = mode( { 'NUMBERS' => \@data } ); # I quote hash keys so I d
+on't 
                                            # mistake them for constan
+ts
                                            # and use them as bareword
+s.
print "Mode : $mode\n";

sub mode {
      my %count = (); 
      my %opt = %{ shift @_ };
      for ( @{ $opt{'NUMBERS'} } ) {  # no need to sort the array here
          $count{ $_ }++;
      }   
      my @key = sort{ $count{$b} <=> $count{$a} } keys %count;
      return $key[0];
}

__END__
[download]

I hope this reply showed you four things:

• A reference to a variable is not the same thing as the variable itself.
• Sometimes taking a reference to a variable is a good thing (e.g., rather than making multiple copies of it), but sometimes it overcomplicates things.
• You should always examine your data if your program is not doing what you expect.
• When you get suggestions from the monks, you should try them out and most likely follow them!
The way forward always starts with a minimal test.