in reply to Re: add missing elements and remove duplicates....and some more questions.
in thread add missing elements and remove duplicates....and some more questions.

Hi,

I think for some reason, it's taking the scalar value of the array as the biggest value. I could be wrong, but just a hunch. I tried again and my script seems to work ok.

use warnings; use strict; my @arr = (10,10,20,0x47,1,30,45,45); sub do_it_all { my $biggest = shift @_; foreach my $num (@_) { if ($num > $biggest) { $biggest = $num; } } my $smallest = shift @_; foreach my $smallnum (@_) { if ($smallnum < $smallest) { $smallest = $smallnum; } } print "\$smallest = $smallest\t\$biggest = $biggest\n"; my @unique = ($smallest..$biggest); print "@unique\n"; } &do_it_all(@arr);

I added the  print "\$smallest = $smallest\t\$biggest = $biggest\n"; line for verification, and the output I get is the following:

C:\>perl no_sort.pl $smallest = 1 $biggest = 71 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 2 +7 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 5 +0 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71
Thinkpad T430 with Ubuntu 16.04.2 running perl 5.24.1 thanks to plenv!!

Replies are listed 'Best First'.
Re^3: add missing elements and remove duplicates....and some more questions.
by AnomalousMonk (Archbishop) on Apr 24, 2017 at 16:34 UTC

    Hi pritesh_ugrankar. If this was posted by you, then you have recognized the problem I wanted to bring to your attention. If it was not, please run the  do_it_all() function given here with the arguments  (1, 2, 3, 4) and see what you get.

    And just as a general observation, code that looks great and doesn't work is worthless. I've written a lot of code that looked wonderful, but...


    Give a man a fish:  <%-{-{-{-<

      Hi,

      No that was not me.

      And I now understand what has happened. So I have changed the code. And you are right, just because my code looks good does not mean it's good.
      use strict; use warnings; my @arr = (1, 2, 3, 4); sub do_it_all { my $biggest = $_[0] ; foreach my $num (@_) { if ($num > $biggest) { $biggest = $num; } } my $smallest = $_[0]; foreach my $smallnum (@_) { if ($smallnum < $smallest) { $smallest = $smallnum; } } print "\$smallest = $smallest\t\$biggest = $biggest\n"; my @unique = ($smallest..$biggest); print "@unique\n"; } &do_it_all(@arr);

      And it now correctly prints:

      C:\>perl anomalous_func.pl $smallest = 1 $biggest = 4 1 2 3 4

      Update: I have updated the main thread with your observation. Thank you very much. I did a stupid mistake that should have been avoided at all costs.

      Thinkpad T430 with Ubuntu 16.04.2 running perl 5.24.1 thanks to plenv!!
        A few additional tips. The indenting of your code levels should be consistent. The "smallest foreach loop" should be moved to the left to match the level of the "biggest". However, better would be to combine the 2 loops. Also Perl has a special single action syntax putting the "if" at the end of the statement. This can make the code easier to read with fewer curly braces required.

        Another big point: What you name variables DOES matter. @unique does not really describe what that is (these are not the uniques of the original array, this is the consecutive range. I would suggest @consecutive as an alternative name.

        use strict; use warnings; my @arr = (1,2,2,3,4,6); sub do_it_all { my $biggest = $_[0]; my $smallest = $_[0]; foreach my $num (@_) { $biggest = $num if ($num > $biggest); $smallest = $num if ($num < $smallest); } print "\$smallest = $smallest\t\$biggest = $biggest\n"; my @consecutive = ($smallest..$biggest); print "@consecutive\n"; } do_it_all(@arr); __END__ $smallest = 1 $biggest = 6 1 2 3 4 5 6
        Update: Instead of "do_it_all", perhaps "conseq_range" or similar would be better?
Re^3: add missing elements and remove duplicates....and some more questions.
by Anonymous Monk on Apr 24, 2017 at 14:15 UTC

    The code looks fine. During edge testing, am having to make 2 line changes.

    my @arr = (1,10,10,20,0x47,30,45,45); sub do_it_all { my $biggest = $_[0]; ... my $smallest = $_[0]; ... }