Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

hi monks!
i want to simplify the step of : if a words is not match in the word array , then do something and add into list box
mm... i try to do that in the following way, but i think it is not a good to do that ,
is there any quicker method to do that?? Thx 
sub addfor{
my ($mainwidget,$entryVariable) = @_;
my $ex='false';
foreach  (@history) {
    if ($_ eq $entryVariable ) {
         $mainwidget->messageBox('-icon' => 'error', -type => 'OK',
         -title => 'Error for input',-message=>"duplicate input");
         $ex= 'true';
    }
}
if ($ex eq 'false'){
$widget{'forwardlistbox'}->insert('end', $entryVariable);    
push (@history, $entryVariable);
}


}

[download]

Replies are listed 'Best First'.
Re: match a word in an words' array
by samtregar (Abbot) on May 02, 2002 at 16:44 UTC
    You could use grep: 
    sub addfor{
   my ($mainwidget,$entryVariable) = @_;
   unless (grep { $_ eq $entryVariable } @history) {
      $widget{'forwardlistbox'}->insert(end => $entryVariable);
      push @history, $entryVariable;
   }
}

    [download]
    This will be slower than necessary since it will keep testing after $entryVariable is found. A faster version could use first() from List::Util: 
    use List::Util 'first';
sub addfor{
   my ($mainwidget,$entryVariable) = @_;
   unless (first { $_ eq $entryVariable } @history) {
      $widget{'forwardlistbox'}->insert(end => $entryVariable);
      push @history, $entryVariable;
   }
}

    [download]
    Note that your code may have problems: I don't see where @history is getting initialized and you're not using $mainWidget in the subroutine. I don't know if these are real problems or not but they might be.

    -sam

[reply]
[d/l]
[select]
Re: match a word in an words' array
by vladb (Vicar) on May 02, 2002 at 16:47 UTC
    Although, unfortunately, I don't quite understand what the problem at hand is or what exactly it is that you are asking, try this code: 
    
sub addfor{
my ($mainwidget,$entryVariable) = @_;
my $ex = 0;
if (grep(/^$entryVariable$/,@history)) {
         $mainwidget->messageBox('-icon' => 'error', -type => 'OK', -t
+itle => 'Error for input',-message=>"duplicate input");
         $ex= 1;
}

unless ($ex){
$widget{'forwardlistbox'}->insert('end', $entryVariable);    
push (@history, $entryVariable);
}
}

    [download]
    Here, I got rid of the foreach() loop and replaced it (and consequent if statement) with a single grep. Also, your original loop would make a message box (annoying pop up i'd guess) appear every time the entry variable is found in the history array. This is no good since also the error message is staying the same all the time.

    Also, you didn't have to use strings for defining true/false state. Just use a simple 1 or 0.

    UPDATE: changed the grep line. Was passing scalar instead of an EXPR. However, this was done long before you've made your reply, samtregar. In fact as soon as I hit the 'Submit' button the first time and spotted the same slip. And excuse me, but that's a pretty rough language you chose to use there buddy!

    UPDATE: Thanks samtregar, it really wasn't my intent to make an 'ass' out of you nor anyone else on PM. I have too much respect for fellow monks to do anything harmful.

    "There is no system but GNU, and Linux is one of its kernels." -- Confession of Faith
[reply]
[d/l]
      Hey, watch your code man! This doesn't work: 
        if (grep($entryVariable,@history)) {

      [download]
      You can't just pass a scalar as the first arg to grep() and expect it to DWYM.

      -sam

      UPDATE: You just changed it without an update comment. Are you trying to make me look like an ass? Don't bother, I've got that covered.

      UPDATE, THE REVENGE: You have my apologies. Just trying to protect the innocent from bad code...

[reply]
[d/l]
Use a hash
by kwilliams (Sexton) on May 03, 2002 at 07:15 UTC
    You should probably be using a hash in the first place. Try something like this: 
    sub addfor{
  my ($mainwidget,$entryVariable) = @_;
  my %history;

  if (exists $history{$entryVariable}) {
    $mainwidget->messageBox
      (-icon => 'error',
       -type => 'OK',
       -title => 'Error for input',
       -message => "duplicate input");
  } else {
    $widget{'forwardlistbox'}->insert
      ('end', $entryVariable);
    $history{$entryVariable} = 1;
  }
}

    [download]
    Hashes are almost always the proper way to implement a set.

    (Also, 'use strict;'! You've got some phantom variables floating around. I'm not sure whether $widget is the same as $mainwidget or not, and @history was unscoped.)

[reply]
[d/l]
Re: match a word in an words' array
by tradez (Pilgrim) on May 02, 2002 at 16:40 UTC
    perldoc -f grep

    Tradez
    "Never underestimate the predicability of stupidity"
    - Bullet Tooth Tony, Snatch (2001)
[reply]
      why is this voted down? was I not clear enough? Sometimes I wonder about you guys.

      Tradez
      "Never underestimate the predicability of stupidity"
      - Bullet Tooth Tony, Snatch (2001)
[reply]

        Well, I can't tell you why anyone chose to vote one way or another, and I haven't voted on this thread at all. But you ask a fair question, so I hope giving you my guess will help.

        Your answer, though clear, is not helpful. If you're going to give someone an "RTFM" answer, at least give them an URL they can follow, like perlfunc:grep or grep. The questioner may not know how to use perldoc at all. It looks like the questioner went to some effort to ask his/her question reasonably clearly, provide a code sample, and so on, but your response is just a throwaway. It looks demeaning; I don't know if that was your intention, but it comes off that way.

        Read 1st monasterians to remember the spirit of how to answer questions. If you can't/don't want to put some effort into a good answer, that's OK. Someone else will step in. But a put-down answer doesn't help anyone, you or the questioner.

[reply]
[d/l]

Back to Seekers of Perl Wisdom