I think a good first step would be to clean up your logic a bit. You've got two while ($all_the_numbers<20) loops, but $all_the_numbers is initialized as a random number from 0 to 18 and never changed - so those are essentially infinite loops. You exit them via last and the loop counters $i and $h, so really, those two loops can be replaced by for loops. More specifically, in the following I've used the more Perlish foreach loop with a range operator, and since you only use the variables $i and $h to ensure the loop runs $all_the_numbers times, I've removed those variable names entirely. The new loops have an added benefit: in your original code, if $all_the_numbers was zero, your code would drop into an infinite loop - maybe you can see why? (Hint: Add a say $i; right before the comparison to $all_the_numbers.)
#!/usr/bin/env perl
use warnings;
use strict;
use feature qw/say/;
my $all_the_numbers=int(rand(19));
say "$all_the_numbers";
my @rand=();
for (1..$all_the_numbers) {
push @rand, 20-int(rand(30));
}
say "@rand";
for (1..$all_the_numbers) {
for (@rand) {
if ( $rand[0]<0 ) { next; } # skip the negative ones
shift @rand;
}
}
say "@rand";
Now, I hope that's easier to read. As for your logic: Note that shift removes the first item off the list, not the current one. If you wanted to do that, you'd need to do something like splice, however, that's tricky because it edits the array you're currently looping over, changing the indices. You said "this resoults in deleting only the positive numbers" - notice that your logic currently says: "Loop over the elements of @rand, each time checking if the first element of @rand is less than zero, and if it is, skip the rest of the loop (i.e. the shift)". Skipping the rest of the loop can also be expressed as follows*: "Loop over the elements of @rand, each time testing the first element of @rand, if that element is greater than or equal to zero, remove it from the array." I hope this explanation helps make it clear what is going wrong?
Now, as for how to fix it, I think I have to suggest re-thinking your overall logic. This is one of those cases where working out the issue on paper would be beneficial, especially if you are required to operate on the array in-place. For each iteration of the loop, keep track (on paper) of the loop counter, the contents of the array, which item you want to remove, and so on. If you are not required to operate on the array in-place, then Perl provides a handy function to make the task of keeping only certain items of an array very easy - have a look at grep.
* Update: Breaking that down:
| [reply]
[d/l]
[select] |
Hello Eso. The first thing I notice in your code is that you loop with the condition $all_the_numbers<20, but $all_the_numbers never changes, meaning you have to exit the loop manually with last (last should mostly be used for early exit, not for the normal stopping condition). The more perlish way to loop a given number of times is:
for (0..$all_the_numbers)
{
... # This will be called $all_the_numbers+1 times
# (eg, if $all_the_numbers is 1, it will be called for 0 and 1,
+so twice)
}
About what you are trying to do: it's simpler to avoid modifying an array while you are iterating over it (looping on it's elements). So you should build the second list in a separate array, which you can either do at the same time as the first one, or in a separate loop, like this:
for (0..$all_the_numbers)
{
my $number = 30 - int(rand(20));
push @all_rand, $number; # Always add to all_rand;
next if $number >= 0; # Skip if number is positive
push @negative_only;
}
Or first build @all_rand, then in a separate loop:
for my $random_number (@all_rand)
{
push @negative_only if $random_number < 0;
}
You still have to declare the variables, and correct the +1 offset on $all_the_numbers. | [reply]
[d/l]
[select] |
This is a problem where it really, really pays off to familiarize with some Perl idioms. The other comments showed steps to get to a result beginning with the code - here's another approach beginning with your problem.
Whenever I see a for loop containing a push, the map function springs into my mind. Together with grep to remove unwanted elements from a list, the code collapses to a few lines (Note that I also dropped use feature qw/say/; because that comes automatically with use 5.10.1;):
#!/usr/bin/perl
use strict;
use warnings;
use 5.10.1;
my $all_the_numbers=int(rand(19));
my @rand = map { 20-int(rand(30)) } (1..$all_the_numbers);
say "@rand";
@rand = grep { $_ < 0 } @rand;
say "@rand";
The function map takes a list to the right (note how I wrote that list without a loop), and applies the code block to each of the elements, to create a new list. In our case, we just create a new random number per element.
The grep function checks a code block against each element of a list and returns a new list containing only those elements where the code block evaluates to a true value. $_ is Perl's magical variable which holds the current value of the list while walking through it (To be precise, it is an alias to the list element, but this doesn't matter in our case).
Edit: Fixed the grep condition as detected by hdb in Re^2: Deleting certein elements from an array. Thanks for spotting this!
| [reply]
[d/l] |
@rand = grep { $_ < 0 } @rand;
| [reply]
[d/l] |
Drats... I fixed it in my test file but forgot to paste back. Thanks! I'll fix it.
| [reply] |
#!/usr/bin/perl
# https://perlmonks.org/?node_id=1229524
use strict;
use warnings;
my @rand = grep $_ < 0, map 20 - int rand 30, 1 .. rand 20;
print "@rand\n";
| [reply]
[d/l] |
To push tybalt89's very good and concise solution one step further, you can make it a single line of code, as in the following Perl one-liner:
$ perl -wE 'say join " ", grep $_ < 0, map 20 - int rand 30, 1 .. rand
+ 20;'
-6 -9 -2
| [reply]
[d/l] |
