|
Re: Why does my get_max_index function return zero? (High Water Mark Algorithm)
by haukex (Archbishop) on Jun 03, 2019 at 17:04 UTC
|
If I try to run the code you've shown* with strict and warnings, it does not compile. But both if I leave them off, and if I turn them on and change my $i, $imax; to my ($i, $imax); as it should be (the second actually declares two lexical variables), then the snippet you've provided works correctly for me, so this code does not actually reproduce the problem you are describing. This is why providing a representative Short, Self-Contained, Correct Example is important.
If I had to guess, maybe you're using a variable named $imax somewhere else in your code, and because of this, the $imax you're using in get_max_index is actually not local to that sub, and so some overlap is going wrong there. Always Use strict and warnings!
* Update: Code has been edited, original code that I was referring to is here.
| [reply]
[d/l]
[select] |
|
|
Sorry, I copy-pasted the original script's code (which didn't use warnings or strict). Please see my edited code above because my script is still returning zero, even when I run the function as its own perl script (I just run the above code in a file called test_max.pl). Also, I don't use $imax anywhere else in the original script that uses the function "get_max_index".
| [reply] |
|
|
The $imax inside your for loop is different from the $imax outside it (as you've declared it with my inside the for loop). Remove that my.
| [reply]
[d/l]
[select] |
|
|
Please see my edited code above
Please have a look at How do I change/delete my post?, in particular, "It is uncool to update a node in a way that renders replies confusing or meaningless". But luckily I still had your original code sitting around:
Other than that, Paladin has already pointed out the issue with the second my declaration inside the loop in your updated code. I just wanted to add that in general, one should not combine my with an if statement modifier, as its behavior is undefined (see Statement Modifiers).
| [reply]
[d/l]
[select] |
Re: Why does my get_max_index function return zero? (High Water Mark Algorithm)
by bliako (Monsignor) on Jun 03, 2019 at 18:48 UTC
|
because you mask original $imax inside the loop with my $imax=$i...
Below is a more esoteric way to do it without a function. It goes under the "Schwartzian transform":
my @arr = (1,2,3,5,10,3,4,300,1,2,-2);
my ($i) = map { $_->[0] } sort { $b->[1] <=> $a->[1] } map { [ $_ , $a
+rr[$_] ] } 0..$#arr;
print "i=$i\n"; # index 7 => value 300
starting from right to left, create an array of all the indices in your array, package the index and its value into a temporary arrayref and then into an intermediate array, sort that intermediate array with custom sort function which sorts in reverse (i.e. biggest first) wrt the second element in said packaged arrayref (which is the value). Remember index and value are still packaged together, so extract the index (first item of the arrayref) with the leftmost map which returns an array of indices of the items in your original array sorted by their value. However my ($i) = ... will retain only the first item (the index with largest value) and skip the rest.
sort of...
bw bliako
Edit: changed $#arr-1 to $#arr, thanks AnomalousMonk | [reply]
[d/l]
[select] |
|
|
c:\@Work\Perl\monks>perl -wMstrict -le
"my @arr = (1,2,3,5,10,3,4,300,1,2,-2);
my ($i) = map { $_->[0] } sort { $b->[1] <=> $a->[1] } map { [ $_ , $
+arr[$_] ] } 0..$#arr-1;
print qq{i of largest == $i (\@arr[$i] == $arr[$i])};
"
i of largest == 7 (@arr[7] == 300)
c:\@Work\Perl\monks>perl -wMstrict -le
"my @arr = (1,2,3,5,10,3,4,300,1,2,-2, 999);
my ($i) = map { $_->[0] } sort { $b->[1] <=> $a->[1] } map { [ $_ , $
+arr[$_] ] } 0..$#arr-1;
print qq{i of largest == $i (\@arr[$i] == $arr[$i])};
"
i of largest == 7 (@arr[7] == 300)
(I'll pass over the propriety of using an O(n log n) sort operation for what is essentially an O(n) task :)
Give a man a fish: <%-{-{-{-<
| [reply]
[d/l]
[select] |
|
|
(I'll pass over the propriety of using an O(n log n) sort operation for what is essentially an O(n) task :)
very good point | [reply]
[d/l] |
|
|
|
|
Re: Why does my get_max_index function return zero? (High Water Mark Algorithm)
by karlgoethebier (Abbot) on Jun 04, 2019 at 12:20 UTC
|
#!/usr/bin/env perl
use strict;
use warnings;
use feature qw(say);
use List::Util qw(max shuffle);
use List::MoreUtils qw(onlyidx);
use Data::Dump;
my @array = shuffle (1..10);
dd \@array;
my $max = max @array;
say $max;
say onlyidx {$_== $max} @array;
__END__
Karls-Mac-mini:Desktop karl$ ./watermark.pl
[7, 10, 9, 8, 2, 6, 4, 3, 5, 1]
10
1
I just cannibalized some fragments i found on my desktop in a hurry. But as far as i remember List::Util and List::MoreUtils come with implementations in pure Perl which might be good for further studies.
Best regards, Karl
«The Crux of the Biscuit is the Apostrophe»
perl -MCrypt::CBC -E 'say Crypt::CBC->new(-key=>'kgb',-cipher=>"Blowfish")->decrypt_hex($ENV{KARL});'Help
| [reply]
[d/l]
[select] |
Re: Why does my get_max_index function return zero? (High Water Mark Algorithm)
by Marshall (Canon) on Jun 04, 2019 at 15:24 UTC
|
I presume that you have gotten your code working based upon comments so far?
If not, then, summarizing...
Update: Well, eating some crow...looks like I goofed on the problem requirements. The advice I give here is good, but it is for a different problem! Darn. I re-coding the variant that passes an array reference in a thread further down. It makes the most sense to use an index if the problem requires an index to be returned.
sub get_max_index { #revisit how to define functions/"my" var ? (defa
+ult parameters?)
my $imax=0;
for (my $i=0; $i<@_; $i++)
{
$imax=$_[$i] if ($imax<=$_[$i]);}
return $imax;
}
I will make a few other comments:
- You are using @_ directly. And yes that works, however, in general this is not a good idea. Usually it is better to make a copy of whatever you want from @_ and then use that copy (or those copies) within your sub. @_ is an alias and if you modify that critter in the sub, you are changing the array in the calling program. This can lead to confusion and mistakes. There can be performance reasons to access @_ directly and the List::Util package does so for little functions like this. However, that is like a 0.0001% case.
- The use of subscripts to access array elements is just not done in situations like this (entire array accessed sequentially). Perl has wonderful array iterators that avoid the possibility of an "off by one" subscript mistake - one of the if not the most common error in programming.
So having said that, I offer this code:
use strict;
use warnings;
sub get_max_index {
my (@copyOfArray) = @_; #make an explicit local copy
my $imax= shift @copyOfArray; # modifies this copy, but ok
for (@copyOfArray){ # no indices...
$imax = $_ if $imax < $_;
}
return $imax;
}
my @arr = (1..10);
my $ans = get_max_index(@arr);
print"$ans\n";
As a further possibility for your study is a version where a reference to @arr is passed to the subroutine. This is faster than the previous because only a single value is passed to the sub instead of all 10 elements. You will note that this version does one more comparison than absolutely necessary. That is due to the way that the loop is initialized. Tradeoffs where a slight performance hit results in much cleaner source code abound. Here is what that looks like:
use strict;
use warnings;
sub get_max_index
{
my $arrayRef = shift;
my $imax = $arrayRef->[0]; #don't modify @arr
for (@$arrayRef){
$imax = $_ if $imax < $_;
}
return $imax;
}
my @arr = (1..10);
my $ans = get_max_index(\@arr); #pass reference to array!
print"$ans\n";
Now of course the "right" way to implement this function is not to implement it at all and use the version in List::Util as pointed out by
karlgoethebier. I see some Monks offered sorting solutions. Yes that works but is way less efficient than linearly scanning for the max and/or min (can do both at the same time).
I hope that my examples are understandable to you that they help you upon your Perl journey. Even a very small section of code can have a lot of Perl "fine points". Wish you well! | [reply]
[d/l]
[select] |
|
|
| [reply]
[d/l] |
|
|
What?? it is supposed to return the next to the highest? Oh... My gosh.......this thing wants the index of highest value, not the highest value itself!! Should have read more carefully.. That of course changes things Ooops...Do not write code before consuming at least one full cup of coffee!
Update:
Ok, if the problem statement requires returning an index, then the most natural formulation would be to use indices. Here is how that looks when a reference to the array is passed to the sub. I just completely blew it and missed that key piece of info, probably because working with an index is a very rare in my coding. More usual in my problem space might be to return a reference to an entire row of a multidimensional array that matches some criteria. additional comment: I remember needing indices when working with some kinds of tk widgets. So a requirement for this sort of thing definitely exists.
use strict;
use warnings;
sub get_max_index
{
my $arrayRef = shift;
my $imax = 0;
for my $i (0..@$arrayRef-1){
$imax = $i if $arrayRef->[$i] > $arrayRef->[$imax];
}
return $imax;
}
my @arr = (1..10);
my $ans = get_max_index(\@arr); #pass reference to array!
print"$ans\n";
| [reply]
[d/l] |
|
|
|
|
|
Re: Why does my get_max_index function return zero? (High Water Mark Algorithm)
by holli (Abbot) on Jun 03, 2019 at 17:05 UTC
|
| [reply]
[d/l] |
|
|
$imax=$i if ($_[$i] > $imax);
No, sorry, that doesn't make sense.
Update: Oh, I see you noticed it too, sorry :-)
| [reply]
[d/l]
[select] |
|
|
| [reply]
[d/l] |
|
|
Hello, I've followed the other respondent's advice, and I'm still getting the same error. Could you look at my updated code?
| [reply] |
|
|
| [reply]
[d/l]
[select] |