Re^2: I have a list expanding when I don't want it to

[Lady_Aleena]

So, undef before I return or check it for definedness before I fill it?

Update: I emptied the two keys before I returned, and it seems to work.

sub random {
  my ($user_input, $list) = @_;
  
  $list->{'all'}  = [ map { @$_ } values %{$list} ];
  $list->{'keys'} = [ grep {$_ !~ /(?:all|keys)/} keys %{$list} ];
  
  my $random_thing;
  if ($user_input && $user_input =~ /(?:help|options)/) {
    $random_thing = 'Your options are: '.join(', ',sort @{$$list{'keys
+'}}).', or all.';
  }
  elsif ($user_input && $user_input =~ /dump/) {
    use Data::Dumper;
    $random_thing = Dumper($list);
  }
  else {
    my $input = $user_input ? $user_input : 'all';
    my @random_list = shuffle(@{$$list{$input}});
    $random_thing = $random_list[rand @random_list];
  }
  
  $list->{'all'} = [];
  $list->{'keys'} = [];
  
  return $random_thing;
}

[BrowserUk]

That's one way to do it. Or you could just delete the keys you don't want (re)included before you (re)populate them:

sub random {
   my ($user_input, $list) = @_;
  
   delete $list->{all}; delete $list->{keys};

   $list->{'all'}  = [ map { @$_ } values %{$list} ];
   $list->{'keys'} = [ grep {$_ !~ /(?:all|keys)/} keys %{$list} ];

[Lady_Aleena]

I thought about using delete, however the doc suggested emptying the lists would be faster. If so I would prefer speed since I may be using these many times (100s of times even) in a script.

[GrandFather]

First make it work (using the easiest to maintain code you can) then, only if you need to, make it faster.

"Need to" requires that you have performed some sort of profiling so you know where things are slow. And before you "speed it up" write unit tests to make sure your speed up isn't at the expense of breaking the code (doing nothing can often be quite fast).

[poj]

Why not leave %list as is ?

sub random {
  my ($user_input, $list) = @_;
  
  if ($user_input =~ /(?:help|options)/) { 
     my $keys =  join ", ",keys %$list;
     return "Your options are: $keys, or all.";
  }
  elsif ($user_input eq 'dump') {
    use Data::Dumper;
    return Dumper($list);
  }
  else {
    my @values=();
    if ($user_input eq 'all'){
      @values = map { @$_ } values %$list;
    } else {
      @values = values @$list{$user_input};
    }
    return (shuffle @values)[0];
  }    
}

poj

[Lady_Aleena]

poj, I did it your way (with my own additions) by not creating two new hash keys.

use List::Util qw(shuffle);

sub random {
  my ($user_input, $list) = @_;
  
  my $random_thing;

  if ($user_input && $user_input =~ /(?:help|options)/) {
    my $keys = join(', ', keys %{$list});
    $random_thing = "Your options are: $keys, or all.";
  }

  elsif ($user_input && $user_input eq 'list') {
    $random_thing = $list;
  }

  else {
    my @random_list;

    # The following gets the keys to randomize instead of $list->{'key
+s'}.
    if ($user_input && $user_input eq 'keys') {
      @random_list = keys %{$list};
    }

    # The following gets the entire list to randomize instead of $list
+->{'all'}.
    elsif (!$user_input || $user_input eq 'all' ) {
      @random_list = map { @$_ } values %{$list};
    }

    else {
      @random_list = shuffle(@{$$list{$user_input}});
    }

    $random_thing = $random_list[rand @random_list];
  }
  
  return $random_thing;
}

[Lady_Aleena]

poj, can you accept I didn't think about doing it your way? Also, you don't have keys available to random generate nor did you make 'all' the default if no $user_input. I sometimes want a more general result which is why I have a keys list. I will give this a big think and maybe incorporate some of this. Thanks!