in reply to Re^4: String comparison in an array
in thread String comparison in an array

There are quite a lot of things to correct here... I'll only point out some of thems because otherwise I'll just end up rewriting everything, and I'd rather leave you with your own code, but feel free to post the cleaned up version again if you want more advice.

First, you might notice that you open two files using two different methods. The second one - where you use a variable with a $, and indicate that the file is for reading with < - is the one to use. The first one is especially bad because DATA is already used by perl to mean something else, so you should really avoid it.

Second, a short and working example is supposed to be short. Remove all the commented lines that make it really hard to read your code (you should keep the useful comments, like # need to change this loop)

Third, this:

print "\n$fillarr[0]\t$fillarr[1]\t$fillarr[3]\t$fillarr[4]\t$fillarr[ +5]\t$fillarr[14]\t";
does mostly the same as:
print "\n", join "\t", @fillarr[0, 1, 3, 4, 5, 14];
(Still not perfect, but better). Or Text::CSV might also help you write as a tab separated file.

Fourth: you can remove the else { next; } because next basically means "stop working on that element", but there's no work to be done at that point (nothing after the else).

Fifth: just including a module doesn't magically solve your problems. You can create your reader like this: my $csv = Text::CSV->new ({ binary => 1, sep_char => ";" }); and then use the parse() and fields() method to get your arrays. Or if you don't want to use Text::CSV, don't include it.

Replies are listed 'Best First'.
Re^6: String comparison in an array
by newperlbie (Acolyte) on Aug 08, 2018 at 14:16 UTC
    Thanks Eily for the corrections....it helps as I am real beginner.I have updated the code with few of your comments,please help
    use strict; use warnings; use Data::Dump; use Text::CSV; use List::MoreUtils qw{ any }; my $first = 'computer'; my $last = 'printer'; my $in = 0; my @keys; open FILE1, "types.txt" or die $!; while (<FILE1>) { if (defined(my $key = (split)[-5])) { if ($key eq $first .. $key eq $last) { $in = 1; push @keys, $key; } elsif ($in && $key eq $last) { push @keys, $key; } else { $in = 0; } } } dd @keys; my $filename = "orders.csv"; my $csv = Text::CSV->new ( { binary => 1 } ); open my $fh, "<:encoding(Latin-1)", "orders.csv" or die "Cannot open $ +filename"; my $do_skip_test = 1; while(my $line = <$fh>) { if ($do_skip_test) { my (@fillarr1) = split ';',$line; if ((grep {$fillarr1[14] eq $_ +}@keys)) #this is bothering me, as hardcoded comparison like $fillarr +1[14] =~ /computer/ is successful { #$do_skip_test = 0; my (@fillarr) = split ';',$line; print "\n", join "\t", @fillarr[0, 1, 3, 4, 5, 14]; } } } close($fh);

      I have updated the code with few of your comments
      The solution was in the last one though. Your analysis was correct (at least it's part of the issue), the fact that @fillarr contains quotes ("computer" rather than just computer) makes the comparison fail. You should create your $csv object like this: my $csv = Text::CSV->new ({ binary => 1, sep_char => ";" }); (the sep_char part is required). And then replace my (@fillarr) = split ';',$line; by using $csv->parse on $line, and then using $csv->fields.

      Also please, replace your first open by open my $types, "<", "types.txt" or die $!; then read with <$types>

      And even if the code doesn't work as you want, please make sure that the code you post here at least compiles ;-)

      Edit: oh and LanX is right, it would be far easier for us to help you with data (ie, an example of what your two files look like). But that's part of what you need for a short, working example (so again, what hippo asked).

        Thanks Eily for the explanation.I am going to clean my code. Meanwhile I tried this and it worked....but is this really a bad piece of code?when there is a huge file?
        foreach(@keys) { if ( $fillarr1[14] =~ /$_/) { #Found }
      This code looks very strange to me.
      if ($key eq $first .. $key eq $last) { $in = 1; push @keys, $key; } elsif ($in && $key eq $last) { push @keys, $key; } else { $in = 0; }
      One purpose of the "flip-flop" operator is to elminate the need for a flag like $in! The flip-flop operator maintains such a flag internally that is not immediately apparent (and doesn't need to be).

      The "if" statement will evaluate to false until $first is seen and remain true up to and including when $last is seen because the flip-flop operator is maintaining the state information that $first has been seen. You do not need to have such a flag yourself.

      if ($key eq $first ... $key eq $last) { push @keys, $key; if ($key eq $last) { # do something to process this record in @keys # assuming that there will be a new record # starting with $first on a subsequent line } }
      I suggest this classic thread Flipin good, or a total flop? for further study. There is a ".." and a "..." version.