CGI/Perl Guide | Learning Center | Forums | Advertise | Login
Site Search: in

  Main Index MAIN
INDEX
Search Posts SEARCH
POSTS
Who's Online WHO'S
ONLINE
Log in LOG
IN

Home: Perl Programming Help: Beginner:
Problem with if condition.

 



Tejas
User

Apr 28, 2014, 9:05 AM

Post #1 of 9 (3985 views)
Problem with if condition. Can't Post

Hi



Code
while(my $line = <IN>) 
{
chomp($line);
my ($Document_Source,$Journal_Category,$Line_Desc) = (split "\t",$line)[4,5,7];

#Automated Entries
if($hash{$Document_Source}{$Journal_Category}{$Line_Desc}){
print OUT "$line.\t$hash{$Document_Source}{$Journal_Category}{$Line_Desc}";
}
else
{
Edge_Cases($line);
}
}


sub Edge_Cases
{
my @tag;
my $edge_case = shift;
chomp $edge_case;
our $tagged = $edge_case."\tTBD\n";
if ($edge_case =~ (/\tSpreadsheet\t/)){$tagged = $edge_case."\tSpreadsheet\tManual\tManual\tTBD\tTBD\n";}
if ($edge_case =~ (/\tSpreadsheet\t/ && (/_001/ && (/GL111/i || /GL351/i)))) {$tagged= $edge_case."\tSpreadsheet\tManual\tManual GL/\ Automated SL\teBooks Revenue\n";}
if ($edge_case =~ (/\tSpreadsheet\t/ && (/PMT_JP/ && /COD/ && (/Adjustment/i || /Trueup/i || /True-up/i))) ) {$tagged= $edge_case."\tSpreadsheet\tManual\tManual GL/\ No SL\tCOD Manual Adjustment\n";}#Other Clumns
.
.
.
.
.
if ($edge_case =~ (/\tSpreadsheet\t/ && (/IGC/ && /Issued/i && /^65/ && /CVS/ && /EGC_REFUND/)) ) {$tagged= $edge_case."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tIGC Issued, CVS EGC Refund\n";}
print $tagged; //This always prints the values of last if condition, no matter if it matched the first
push @tag,$tagged; //This is nt getting copied
}
print OUT @tag // This is nt printing anything

close IN
CLOSE OUT



Quote
This code always matches the last if conidtion, cant understand why.
Also @tag doesnt have any value.
Is there a problem with pattern matching in this implementaion ?
All th eif conditions are getting satisfie,not sure why :(



(This post was edited by Tejas on Apr 28, 2014, 9:20 AM)


BillKSmith
Veteran

Apr 28, 2014, 10:35 AM

Post #2 of 9 (3955 views)
Re: [Tejas] Problem with if condition. [In reply to] Can't Post

strict and warnings would tell you why you are not getting any output to OUT (scope to @tag).

You do not know if anything is copied because of previous error.

Parenthesis do not mean the same thing in a regex as they do in perl. I am not sure that I can parse your regex's correctly, but I am quite sure that they do not mean what you intend. It would be much easier to test the default $_.
Good Luck,
Bill


Laurent_R
Veteran / Moderator

Apr 28, 2014, 2:28 PM

Post #3 of 9 (3944 views)
Re: [Tejas] Problem with if condition. [In reply to] Can't Post

Hi,
1. @tag is out of scope when you try to print it (it is declared within the function and the print statement is outside it). Actually, each time you call the function, a new empty version of @tag is created; the push at the end of your function is actually copying $tagged into @tag, but the @tag array is lost immediately after when you exit the function. As Bill said, you would have had an error message telling you that if you had used use strict and use warnings. You might probably also have seen it if you had indented your code correctly.

2. All your successive if conditions are not mutually exclusive. So if the last one is true, any previous value of $tagged is lost and $tagged will take the value assigned to it in the last if condition, even if other conditions were satisfied. This may or may not be what you really want. Using elsif and else statements might make your logics clearer.

3. Many or possibly all your conditions have the /\tSpreadsheet\t/ regex in them, may be this should be factored out in nested conditionals to make the rest easier to understand.

4. I do not see any reason to declare #tagged a global variable with the our statement, a "my" lexical variable would do just as good and probably better.

5. No need to chomp $edge_case, since $line was already chomped.

6. '/Trueup/i || /True-up/i' can be rewritten '/True-?up/i', making the conditional simpler.

7. I very much doubt that the code you presented compiles. In particular, this:

Code
print OUT @tag  // This is nt printing anything  

close IN
CLOSE OUT

will not compile because:
- // is not something introducing comments, you would need #;
- there is no semi-colon between these three lines of code;
- I do not know any Perl operator called CLOSE in uppercase.

Please provide code that at least compiles (unless your question is why it does not compile, but that's not your question).

As for your real question, I made some assumptions or hypotheses to try to give partial answers, but, if you want more, then you'll have to supply input data: I can't give answers based on guesses about what is in the data.


(This post was edited by Laurent_R on Apr 28, 2014, 2:29 PM)


Tejas
User

Apr 29, 2014, 6:41 AM

Post #4 of 9 (3638 views)
Re: [Laurent_R] Problem with if condition. [In reply to] Can't Post

Hi

I would like to follow a best approach for the below if conditions
This actually takes data from a file and directly searchs.
Well,This is code we are using
And i would like to reimplement it..


Code
use strict; 
use warnings;
my $start_run = time();

open(my $in, '<Spreadsheet.txt') || die;
my @tag;
while (<$in>) {
our $line = $_;
chomp $line;
our $tagged = $line."\tTBD\n";

if (/\tSpreadsheet\t/ && (/Internal Redeem/i)) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Redeemed\n";}
if (/\tSpreadsheet\t/ && (/Internal/ && (/GIVEIT-FOR-RETURN/i || /RefForRet/i || /Giveit For Return/i))) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Giveit-for-Return\n";}
if (/\tSpreadsheet\t/ && (/Internal/ && (/GIVEIT-TO-RECIPIENT/i || /GIVEIT TO RECIPIENT/i || /Giveit to Reci/i))) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Giveit-to-Recipient\n";}
if (/\tSpreadsheet\t/ && (/Internal/ && (/RTZ/i || /Purchaser/i) && ! /Recipient/i)) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Giveit-to-Purchaser\n";}
if (/\tSpreadsheet\t/ && (/Internal/ && (/GIFT_RECI/i || /Gift Recipient/i))) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Issued to Gift Recipient\n";}
if (/\tSpreadsheet\t/ && (/Internal/ && /Issued/i && (! /^65/ || (/^65/ && ! /DOC/ && ! /VSC/)))) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Issued\n";}
if (/\tSpreadsheet\t/ && (/Internal/ && /Revoke/i)) {$tagged = $line."\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Revoke\n";}
print $tagged;

}


Attached is the input file with tab seperated values and the above if conditions are being checked on whole line
Most of them on fourth and seventh column(1st to 5th if conditions ) of the input file
Zeroth,fourth and seventh column(6th to 7th if conditions )
I tried to create a dispatch table for Spreadsheet,but got confused to the core .
Can someone show me the way and method's,so that i can improve this code.
Attachments: Spreadsheet1.txt (3.64 KB)


FishMonger
Veteran / Moderator

Apr 29, 2014, 8:48 AM

Post #5 of 9 (3601 views)
Re: [Tejas] Problem with if condition. [In reply to] Can't Post

That code is extremely inefficient and you're ignoring several suggestions/recommendations that I gave in your other questions related to this task.

DON'T use global vars unless you REALLY need them, which you don't.

DON'T put the if blocks on single lines like that. It makes the code LESS readable and LESS maintainable.

DON'T apply multiple regexs to the entire line. Instead, you should split the line into its individual fields and apply the regexs to the desired fields. You can do the split yourself, or use the Text::CSV_XS module and let it handle splitting the fields.

Each of your conditional tests is checking for Spreadsheet which is the 5th field (index 4). You should only need to do that test once for each line, not for every different sub condition. The same goes for the test for Internal.

You should jump to the next iteration of the loop as early as possible so that you don't waste time & cpu cycles on conditionals that don't need to be applied.


Code
#!/usr/bin/perl 

use strict;
use warnings;

my $file = 'Spreadsheet.txt';
open(my $fh, '<', $file) or die "Failed to open '$file' <$!>";

LINE: while (my $line = <$fh>) {
chomp $line;
my ($col_1, $col_5, $col_8) = (split /\t/, $line)[0,4,7];

if ($col_5 ne 'Spreadsheet') {
print "$line\tTBD\n";
next LINE;
}
# We now no longer need to test for "Spreadsheet"

if ($col_8 =~ /Internal/) {

# and now, we no longer need to test for "Internal"

# normally I'd use a join statement to build the line to be printed
# but for now I kept the long string you were using.
if ( $col_8 =~ /Redeam/i ) {
print "$line\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Redeemed\n";
}
elsif ( $col_8 =~ /(GIVEIT[ -]FOR[ -]RETURN|RefForRet)/i ) {
print "$line\tSpreadsheet\tManual\tManual GL/\ Automated SL\tInternal Giveit-for-Return\n";
}
elsif ( $col_8 =~ /GIVEIT-TO-RECIPIENT/i ) {
# here's how I'd use the join statement I mentioned
print join("\t", $line,
'Spreadsheet', # why add this 2nd Spreadsheet field?
'Manual',
'Manual GL/\ Automated SL',
'Internal Giveit-to-Recipient'
) . "\n";
}
# etc
}
}



Tejas
User

Apr 29, 2014, 9:04 AM

Post #6 of 9 (3592 views)
Re: [FishMonger] Problem with if condition. [In reply to] Can't Post

Hi

Thanks for the suggestion
This code has been implemented by an ex-colleague , and ias it is confusing , i would want ot reimplement it in the best way possible

I tried Regexp Hashes, Normal Hashing and Disatch tables too.

But found it tough to implement it as some if conditions are really deep

Code
( $line =~ /\tSpreadsheet\t/ && (/_001/ && (/ Software/i || /Digital /i) && (/Getit/i || /GiveIT/i || /SalePrb/i)))


As per your suggestion , For this i have to read on and on for 6 times i suppose

Your help has been great

Thanks :)


Tejas
User

Apr 29, 2014, 9:22 AM

Post #7 of 9 (3585 views)
Re: [FishMonger] Problem with if condition. [In reply to] Can't Post


In Reply To
print join("\t", $line,
'Spreadsheet', # why add this 2nd Spreadsheet field?
'Manual',
'Manual GL/\ Automated SL',
'Internal Giveit-to-Recipient'
) . "\n";
}
# etc


Thts the way data has to look in excel sheet as per the requirement
So Just appending the string


(This post was edited by Tejas on Apr 29, 2014, 9:23 AM)


FishMonger
Veteran / Moderator

Apr 29, 2014, 9:27 AM

Post #8 of 9 (3580 views)
Re: [Tejas] Problem with if condition. [In reply to] Can't Post

If the output is for creating an excel spreadsheet, then you should use the proper module for that purpose.

Spreadsheet::WriteExcel - Write to a cross-platform Excel binary file.
http://search.cpan.org/~jmcnamara/Spreadsheet-WriteExcel-2.40/lib/Spreadsheet/WriteExcel.pm


Tejas
User

Apr 29, 2014, 9:48 AM

Post #9 of 9 (3572 views)
Re: [FishMonger] Problem with if condition. [In reply to] Can't Post

Sure Sir.
Iam struggling to find out what these if's are upto :) at the moment :).
Terribly Stuck.
Once this is done, i will generate a proper excel.

Thanks for all the help

 
 


Search for (options) Powered by Gossamer Forum v.1.2.0

Web Applications & Managed Hosting Powered by Gossamer Threads
Visit our Mailing List Archives