Nagging Subroutine

[Ramnarayan]

Hi,

I just made a simple subroutine to check whether the zip file has jpg files. However the script is working great. But whenever it tests a zip file that does not have a jpg file, it should do the other part of the loop. This is not working... Can someone help me!

my $has_jpg = "FALSE";
open (JPG, "unzip -t fff.zip | grep jpg |&quot or die "Can't get JPG files from zipfile\n";
while(<JPG>)
{
if (m/OK\s*$/)
{
$has_jpg = &quot;TRUE&quot;;
last;
}
}
close (JPG);
if ($has_jpg = &quot;TRUE&quot
{
&withjpg;
}
elsif ($has_jpg = &quot;FALSE&quot
{
&nojpg;
}
sub withjpg
{
!system(&quot;unzip -qq $fff.zip *.txt *.jpg *.rpt&quot or die (&quot;can't unzip zip file: $!&quot;
}
sub no jpg;
{
!system(&quot;unzip -qq fff.zip *.txt *.rpt&quot or die(&quot;can't unzip zip file: $!&quot;
}

 

[youradds]

Why are you using;

elsif ($has_jpg = &quot;FALSE&quot

Think about it.... $has_jpg won't be set to FALSE....you need to just use an 'else', rather than 'elsif'

That should do it

Cheers

Andy

 

[Ramnarayan]

Andy,

That does not work either. I replaced elsif (....) with else and that too did not work. Please help me out!

 

[carlosAlberto]

From your example above -

sub no jpg;
{
!system(&quot;unzip -qq fff.zip *.txt *.rpt&quot or die(&quot;can't unzip zip file: $!&quot;
}


sub no jpg; - remove the ';' should do it.




Agree with changing the elseif too, good programming practice.


Carl.

 

[youradds]

Also, the nojpg has a space in it

sub no jpg;


Try the following code;

my $has_jpg = "FALSE";
open (JPG, "unzip -t fff.zip | grep jpg |") or die "Can't get JPG files from zipfile\n";
while(<JPG>)
{
    if (m/OK\s*$/)
    {
        $has_jpg = &quot;TRUE&quot;;
        last;
    }
}
close (JPG);
if ($has_jpg = &quot;TRUE&quot;)
{
    &withjpg;
}
else ($has_jpg = &quot;FALSE&quot;)
{
    &nojpg;
}

sub withjpg
{
    !system(&quot;unzip -qq $fff.zip *.txt *.jpg *.rpt&quot;) or die (&quot;can't unzip zip file: $!&quot;);
}

sub nojpg;
{
    !system(&quot;unzip -qq fff.zip *.txt *.rpt&quot;) or die(&quot;can't unzip zip file: $!&quot;);
}

 

[rosenk]

[tt]if ($has_jpg = &quot;TRUE&quot[/tt]

is assigning the string &quot;TRUE&quot; to [tt]$has_jpg[/tt], and returning it. This is always true.

You want:
[tt]if ($has_jpg eq &quot;TRUE&quot[/tt]

 

[youradds]

Good point Carl, you will need to remove the ; off the end of the following line that I gave you;

sub nojpg;

I managed to miss that one

Cheers

Andy

 

[Ramnarayan]

This is my refined code after correcting of all the mistakes mentioned above. Thanks to you all for pointing it out. However, still the script does not work for zip files without jpg files.

my $has_jpg = &quot;FALSE&quot;;
open (JPG, &quot;unzip -t fff.zip | grep jpg |&quot or die &quot;Can't get JPG files from zipfile\n&quot;;
while(<JPG>)
{
if (m/OK\s*$/)
{
$has_jpg = &quot;TRUE&quot;;
last;
}
}
close (JPG);
if ($has_jpg = &quot;TRUE&quot
{
&withjpg;
}
else
{
&nojpg;
}
sub withjpg
{
!system(&quot;unzip -qq $fff.zip *.txt *.jpg *.rpt&quot or die (&quot;can't unzip zip file: $!&quot;
}
sub nojpg
{
!system(&quot;unzip -qq fff.zip *.txt *.rpt&quot or die(&quot;can't unzip zip file: $!&quot;
}

 

[Ramnarayan]

Hey all,

Thanks for the wonderful tips. I got what Rosenk said. It is the problem due to the &quot;=&quot; assigner operation. I changed it to &quot;eq&quot; and the script worked wonders.

Thanks to all for helping me out.

 

[toolkit]

What's with all the exclamation marks in front of your system calls??

 

[Ramnarayan]

Toolkit,

This was told to me during a training session. If there is no reason for me to put the exclamation marks, Please enlighten me. Thanks for pointing it out to me!

 

[toolkit]

Actually you are quite right.
System returns 0 for success, and non zero for failure, so you can use either:

1) system(xxx) and die "yyy";
OR:
2) !system(xxx) or die "yyy";

The 'and' and 'or' here are short circuit logical operators, so in both 1) and 2) the die clause will only get called if system returns non zero (fails). However, there is no value in doing:

!system(xxx)

Sorry if I confused you