On Sat, 28 Sep 2002, Shannon Appelcline wrote:

[snip]
> I start off with a simple index.cgi that I've made extremely short to 
> avoid the issue of nested subroutines. It has a require:
> 
> --
> $main::filePath = "/var/www/skotos/myskotos";
> require "${main::filePath}/modules/lib/mylib.pm";

Shannon,

I'll make some suggestions about style that might help solve your
problems.  First off, I wouldn't recommend explicitly including
package names with your variables ("$main::foo," "$bar::baz," etc.).
You mentioned getting things to work with strict, but the above does
not use "my" for "$filePath," which leads me to believe you might be
declaring this as a global.  And while your "require" is technically
fine, why not just set a "use lib" line for your custom library path
and then "use" your module?  (Your library path doesn't need to be
variable, does it?  Will this change dynamically at run-time?)  I'd
recommend changing the above to:

    use lib '/var/www/skotos/myskotos';
    my $filePath = "/var/www/skotos/myskotos";
    use mylib;

>  From that required library, I set my $cgi variable:
> 
> --
> package mylib;
> 
> use CGI;
> use CGI::Pretty qw( :html3 );
> 
> $mylib::cgi = new CGI;
> --
> 
> It's a global variable, but that seems entirely appropriate for 
> something that's used in nearly every function. And, it gets 
> explicitly set every time the program is run.

But this isn't a good way to set a global.  Either use "use vars" or
"our" (depending on your version of Perl).  And, stylistically, I
prefer to make globals and constants stand out with all-caps:

    package mylib;

    use CGI;
    use CGI::Pretty qw( :html3 );
    use vars '$CGI';

    $CGI = CGI->new;

I don't necessarily agree with making this a global, though.  I don't
use CGI (I usually use the faster and mod_perl-appropriate
"Apache::Request"), so I'd read up on the docs to see if making this a
persistent global isn't causing some of your headaches.  Would it be
that bad to just have that be redeclared every time with a "my"?

> Back in the original index.cgi, I have the problems when it's called 
> to process form information. Here's that particular segment of code:
> 
> --
>      require "${main::filePath}/modules/web/myedit.pm";
> 
>      my $action = $mylib::cgi->param('action');
> 
>      $main::pageNumber = $mylib::cgi->param('pageNumber');
>      $main::contentType = $mylib::cgi->param('contentType');
> 
>      if ($action eq "editpage") {
>          &myedit::ProcessEditPage();
>      } elsif ($action eq "editcontent") {
>          &myedit::ProcessEditContent();
>      }
> --
> 
> And, pretty much ANY of those parameters that I call in from $cgi can 
> come up wrong. Both the local variables defined by my ($action) and 
> the global variables set to $main ($pageNumber, $contentType) ... 
> which tells me that the problem is back in that $cgi reference.
> 
> But why?
> 
> Can I not set variables which might change down in modules? That 
> would seem grossly limiting if so, given that I've moved everything 
> to modules in order to avoid the nested subroutine problems.
> 
> Or am I missing something else?

I'm still on the style issue of explicitly manipulating another
package's variables.  It really seems like you want to make those
extra modules of yours into real objects, and then call methods
against them:

    package mylib;

    use CGI;
    use CGI::Pretty qw( :html3 );

    sub new {
        my $class = shift;
        my $self  = bless { cgi => CGI->new }, $class;
        return $self;
    }

    sub cgi {
        return shift->{'cgi'};
    }

    1;

---------------

    use lib '/var/www/skotos/myskotos/modules/web/';
    use myedit;
    use mylib;

    my $lib         = mylib->new                 or die 'No mylib object';
    my $edit        = myedit->new                or die  'No edit object';
    my $cgi         = $lib->cgi                  or die   'No CGI object';
    my $action      = $cgi->param('action')      ||            'editpage';
    my $pageNumber  = $cgi->param('pageNumber')  ||                     0;
    my $contentType = $cgi->param('contentType') ||           'text/html';

    if ( $action eq 'editpage' ) {
        $edit->ProcessEditPage( $cgi );
    } 
    elsif ( $action eq 'editcontent' ) {
        myedit->ProcessEditContent( $cgi );
    }

Of course, you should be writing code a lot more bullet-proof than the
above (what's going to catch the "die," what happens in "mylib" if
"CGI->new" fails, taint checking, etc.).  My guess is that in your
"myedit" package, you were explicitly referencing "$mylib::cgi"
because I see you weren't passing it (as I made sure to above).
Again, I differ with you on style, but I think this is part of your
problem.

> I should mention, that out of extreme paranoia at this point I'm even 
> trying to undef $cgi when I'm done, to no effect.
> 
> --
> END {
>      $mylib::dbh->disconnect if $mylib::dbh;
>      undef $mylib::cgi;
> }

I don't think undefing your CGI object is the problem.  I think it
probably never gets created anew as you expect it would.  You've
declared it as a global, and then you just keep reusing this class's
global variable.  Why should it change?  I think you need to wrap your
libraries up as objects and reinstantiate them as needed.  This is
probably what will make CGI parse the new request.  But if you're
writing the mod_perl list, then you must be using mod_perl, so go
ahead and start using Apache::Request, instead.  Also, you should only
need one "use lib" statement, and it should go in something like a
Perl startup file parsed at startup by Apache.  Then set your
PERL5LIB env to the same path.  Put your modules into well-defined
sub-dirs from there.  E.g., choose a better name than "mylib," like
your project name, like maybe "HCFM" if you're working for the "Hair
Club for Men."  So your stuff might look like:

    /var/www/skotos/myskotos/modules/web/HCFM.pm
    /var/www/skotos/myskotos/modules/web/HCFM/Edit.pm

Then you do this:

    use lib '/var/www/skotos/myskotos/modules/web/';
    use HCFM;
    use HCFM::Edit;

    my $hcfm = HCFM->new;
    my $edit = HCFM::Edit->new;

Etc.

HTH,

ky

Reply via email to