scoping problem

[ZMAN]

Hello all-

I have a routine to generate an image map that I've written for our HTML
wrapper that we use here at work.
I for the life of me have not been able to clear out the array @t_imaprows.
Keeps appending with each pass.
Any help would be greatle appreciated thanks in advance!


########################################################
my (@t_imaprows)=();

%env_imap = (
'begin_fn' => ['*SKIP*', 'BeginImap'],
'end_fn' => ['*SKIP*', 'EndImap'],
'name' => '',
);

sub mcr_imaprow
{
push(@t_imaprows, {@_});
return(undef);
}
sub BeginImap
{
my($cmp) = &df::CurrentComponent();
my($tag, $pass, %env) = @_;
return(undef) if ($factory:ass_level < 1);
my($r) = '';
my $name = $env{'name'};
$r .= "<map name=\"$name\">\n";
my($i);
for($i = 0; $i <= $#t_imaprows; $i++ )
{
my(%t_imaprow) = %{$t_imaprows[$i]};
my $coords = $t_imaprow{'coords'};
my $shape = $t_imaprow{'shape'};
my $target = $t_imaprow{'target'};
my($link) = $t_imaprow{'link'};
my($l) = &make_link($link);
my($target_str);

if ($t_imaprow{'target'} eq '')
{
$target_str = '';
} else {
$target_str = "target=\"_$setimaprow{'target'}\"";
}
$r .= "<AREA coords=\"$coords\" href=$l shape=\"$shape\"
$target_str>\n";
}

$r .= "</map>\n";

return($r);
}

sub EndImap
{
return(undef);
}

 

[John Bokma]

Hello all-

I have a routine to generate an image map that I've written for our HTML
wrapper that we use here at work.
I for the life of me have not been able to clear out the array @t_imaprows.

Where do you clear it? And why do you use a global?

Keeps appending with each pass.
Any help would be greatle appreciated thanks in advance!


########################################################
my (@t_imaprows)=();

%env_imap = (
'begin_fn' => ['*SKIP*', 'BeginImap'],
'end_fn' => ['*SKIP*', 'EndImap'],
'name' => '',
);

sub mcr_imaprow
{
push(@t_imaprows, {@_});
return(undef);

that's the default behaviour of return so use return;

}
sub BeginImap
{
my($cmp) = &df::CurrentComponent();

No need for the & I guess.

my($tag, $pass, %env) = @_;

return(undef) if ($factory:ass_level < 1);

return if $factory:ass_level < 1;

my($r) = '';
my $name = $env{'name'};
$r .= "<map name=\"$name\">\n";
my($i);
for($i = 0; $i <= $#t_imaprows; $i++ )

for (my $i = 0; etc .)

{
my(%t_imaprow) = %{$t_imaprows[$i]};
my $coords = $t_imaprow{'coords'};
my $shape = $t_imaprow{'shape'};
my $target = $t_imaprow{'target'};
my($link) = $t_imaprow{'link'};
my($l) = &make_link($link);

drop the &

my($target_str);

if ($t_imaprow{'target'} eq '')
{
$target_str = '';
} else {
$target_str = "target=\"_$setimaprow{'target'}\"";
}

my $target_str = $t_imaprow{'target'} eq ''
?
''
:
qq(target="_$setimaprow{'target'}");

$r .= "<AREA coords=\"$coords\" href=$l shape=\"$shape\"
$target_str>\n";

use qq here, no need to escape the ".

}

if you want to clear it, @t_imaprows = (); for example.

 

[Ben Morrow]

that's the default behaviour of return so use return;

No it's not. return with no argument returns undef in scalar ctx and the
empty list in list ctx. This means it is always false. return undef is
true in list ctx (which is not usually what you meant).

Ben

 

[John Bokma]

No it's not. return with no argument returns undef in scalar ctx and the
empty list in list ctx. This means it is always false. return undef is
true in list ctx (which is not usually what you meant).

Oops. I stand corrected, thanks.

 

[Eric Schwartz]

I have a routine to generate an image map that I've written for our HTML
wrapper that we use here at work.
I for the life of me have not been able to clear out the array @t_imaprows.

You should define it as closely as possible to the scope in which it
is used, instead of using a global.

Keeps appending with each pass.
Any help would be greatle appreciated thanks in advance!

I'm going to comment on a number of other things as well.

########################################################
my (@t_imaprows)=();

Don't define this here.

%env_imap = (
'begin_fn' => ['*SKIP*', 'BeginImap'],
'end_fn' => ['*SKIP*', 'EndImap'],
'name' => '',
);

%env_imap is not declared with 'our' or 'my'. Why not? Fix this.

sub mcr_imaprow
{
push(@t_imaprows, {@_});
return(undef);
}

I don't think this is actually needed in this code. Anyway, it's so
massively trivial, I can't see wasint a sub on it. Also, your sub
naming style seems inconsistent-- why use lower_case_with_underscores
here and CamelCase later on? Pick one and stick with it; it's easier
to grasp.

sub BeginImap
{
my($cmp) = &df::CurrentComponent();

Don't use & for function calls unless you know exactly what it does,
because 99% of the time, you don't want its side-effects.

my($tag, $pass, %env) = @_;
return(undef) if ($factory:ass_level < 1);

You probably just want 'return if...' here.

my($r) = '';

"my $r;" will do the same thing, but I don't mind explicit
initialization very much.

my $name = $env{'name'};
$r .= "<map name=\"$name\">\n";

my($i);
for($i = 0; $i <= $#t_imaprows; $i++ )
{
my(%t_imaprow) = %{$t_imaprows[$i]};

This whole bit here can be replaced with:

foreach my $t_imaprow (@t_imaprows) {

and then replace

my $coords = $t_imaprow{'coords'};

with

my $coords = $t_imaprow->{'coords'};

C-style loops are generally frowned on in Perl programs, and they tend
to distract me from the purpose of the loop. Since you're not using
$i anywhere else in the loop, just don't use that style of loop.

my $shape = $t_imaprow{'shape'};
my $target = $t_imaprow{'target'};
my($link) = $t_imaprow{'link'};

Fix all these as above.

my($l) = &make_link($link);

Don't use & here either. Also, $l is a terrible name for a variable.
In some fonts, it's hard to tell the difference between $1 and $l. In
point of fact, below I had to read your code a few times trying to
figure out where you'd used a regex before I realized it was $l, not
$1. Remember, you write code for programmers, not for compilers. It
won't run any faster if you call it $html_link instead of $l.

my($target_str);

This may be a style thing, but I don't define 'my' variables in a list
unless I'm initializing them with a list. Also, you initialized $r to
'' above; if you're going to do it anywhere, at least be consistent--
either fix this definiton or the one for $r above.

if ($t_imaprow{'target'} eq '')
{
$target_str = '';
} else {
$target_str = "target=\"_$setimaprow{'target'}\"";

Use alternate quoting here to save backwhacks:

$target_str = qq{target="_$setimaprow{'target'}"};

perldoc perlop for "Quote and Quote¡¾like Operators"

}
$r .= "<AREA coords=\"$coords\" href=$l shape=\"$shape\"
$target_str>\n";
}

$r .= "</map>\n";

return($r);
}

You never say at what point you want @t_imaprows cleared; right before
the return might be a good point; you could just put

@t_imaprows = ();

there.

Of course, the Right Answer(tm) in my mind is to pass in t_imaprows as
an arrayref and then clear it between calls to BeginImap. The problem
is that you define it as a global variable and then don't show how you
use it outside this context, so we can't really tell you where an
appropriate place to clear it is.

If, instead, you define it as a local variable, then you can do
something like:

my @t_imaprows = GetImapRows();
BeginImap($tag, $pass, \%env, \@t_imaprows);
....
@t_imaprows = (); # empty the variable
....
EndImap()

sub EndImap
{
return(undef);
}

I'll just second others' responses that you should just use 'return;'
here.

From what you've shown there doesn't seem to be a problem with just
assigning an empty list to @t_imaprows at any point you decide it
should be 'cleared', so maybe you should try to create a small (10 to
20 lines long) program that exhibits your problem that we can look at.
Posting your whole program is likely to cause brain hemorrages in most
posters here, and I usually find that in the process of trying to
create a small example that reproduces my problem, I can figure it out
on my own.

-=Eric