Hi Jonathan.

This thread needs splitting. And you need to formulate and ask
one question at a time. There's no use in starting with an
entire program that doesn't work and trying to fix it. Model
each of your problems in a tiny piece of Perl and fix them
one at a time. It will help you as well as ourselves.

Jonathan Mangin wrote:
>
> Thank you for all the responses. I'll try to answer everyone's
> concerns here. This is an inherited project I'm converting
> from cgi-lib.pl to cgi.pm. I'm purposely doing everything the
> hard way in an effort to learn more core perl. Session
> management is next, then converting to C::A. That's the plan.?
>
> > > #!/usr/bin/perl -wT
> > > use strict;
> > >
> > > [CGI and DBI stuff]
> > >
> > > $countryid="1";
>
> $countryid is being declared globally with use vars. It's
> processed and stored where needed but is meaningless at this
> time.

What version of Perl are you running? 'use vars' has been
superseded by 'our' for a long time. Also you need to be sure
that you use 'our' variables as little as possible.

> > > $statename=$gotcha;
> > > @r_states=getStates([EMAIL PROTECTED], \%states1, \%states2);
> > > $stateid=$states2{$statename};
> > > @r_counties=getCounties([EMAIL PROTECTED], \%counties1);
> > >
>
> Best practices or less, I intended (at this time) these
> arrays/hashii to be global. More later...

That doesn't look like a bad decision, as long as you've made a
design choice about exactly /which/ variables should be
global.

> > > if ($CGI->param("Register")) {
> > >    Register($CGI, \%$counties1);
> > > }
>
> Hmm, I truly thought the syntax should be: Register($CGI,
> \%counties1); I cut and pasted one of my fanciful iterations.

Your thoughts were right. But if '%counties1' is a global
variable then it would be very wrong to pass it to a subroutine
in any way.

> > > elsif ($CGI->param("Verify")) {
> > >    Verify($CGI);
> > > }
> > > else {
> > >    displayLoginScreen($CGI);
> > > }
> > > sub displayLoginScreen {
> > >    [misc stuff]
> > >
>
> Let's add to the following for a test:
>
> > >    foreach $state(@states) {
> > >       print "<option>$state";
> > >    }
>
> foreach $county(@counties) {
>    print "<option>$county";
> }
>
> Now the list of states also includes the counties (fetched
> previously based on $stateid).

Hmm. I don't have a panoramic view on your brain and can't
understand this at all.

[snip content]

> >
> > All of the variables that you've declared ouside any
> > subroutines are accessible throughout the rest of the file,
> > both within and outside subroutines. You're simply accessing
> > the common '@states' variable in 'displayLoginScreen'.
> > '@states' and '$states' are totally unrealted: they just
> > happen to have the same identifier. The syntax '@$states' is
> > an attempt to dereference the value in the '$states' scalar
> > as an array. This will fail unless '$states' holds an array
> > reference. I don't understand what you mean by "more
> > persistent" or by 'Verify' knows the county stuff only until
> > the first validation error." You're passing no references to
> > 'Verify' so it can only be accessing the common variables
> > like '@states'.
>
> Subroutine 'Verify' processes the fields from
> displayLoginScreen, validates the input and checks the db for
> existing records. (We're actually attempting a new
> registration here.) i.e.:

Once more, we have no idea what 'attempting a new registration'
means. Remember that we cannot see anything of your system and
your application other than what you tell us. We know Perl plus
most popular modules. Period.

> sub Verify {
>    my $cgi=shift;
>    $uid=$cgi->param(-name=>'uid');
>    if (length($uid) < 6) {
>       displayLoginScreen($cgi);
>    }
>    elsif (nextErrorCondition) {
>       displayLoginScreen($cgi);
>    } else {
>       [more stuff]
>       if ([more stuff] =OK) {
>          displayRegisterScreen($cgi);
>       }
>    }
> }
>
> Upon the first error condition being true (or true again) and
> subroutine displayLoginScreen being [re]called, my list of
> select options still includes @states but not @counties. Are
> these not global, or is @counties being undefined somehow?

You must tell us what you are seeing within Perl. What appears
on your screen will be several levels down from 'Verify', and
any one of those levels may have a problem. If you have a
problem with a single piece of independent code then we can
start to guess.

> > > Instead of making subsequent db fetches I want to send(?)
> > > any/all of these to any/all subsequent (inner?)
> > > subroutines that may need them. Is this possible and what
> > > might the referencing/dereferencing look like?
> >
> > You can either do what you've done above, and simply declare
> > common variables at a wider scope so that they're accessible
> > everywhere or - more properly - pass references to all of
> > the data as as parameters. You'd then have something like
> >
> >   displayLoginScreen($CGI, [EMAIL PROTECTED]);
> >
> >   sub displayLoginScreen {
> >     my ($CGI, $states) = @_;
> >     foreach my $state (@$states) {
> >       :
> >     }
> >   }
> >
> > > Verify calls other subroutines and then another HTML page.
> > > That page calls Register. Do I need to send references to
> > > Verify before sending them to the other subs?
> >
> > If you're expecting the call to the other subroutines to
> > pass all of the data they need, then clearly the calling
> > code has to have access to that data to be able to pass it
> > on. This is where parameterising all data can get messy, and
> > it becomes more appropriate to use common data accessible
> > everywhere, preferably with some naming convention that
> > makes it clear that it is global data.
>
> If I need to explicitly pass data (references) to subroutines
> that's OK cause that's what I'm trying to learn. The @states
> vs. @counties inconsistency has me confused.

Never 'play' with production code. Write hundreds of minimal
programs using the constructs you're not sure about and then,
when you've understood them, write some code, believing it to
be at least 90% reliable. Then debug.

> > but you seemed to know what to do in the previous call to
> > 'getStates', like this.
> >
> >   getStates([EMAIL PROTECTED], \%states1, \%states2)
> >
> > I hope that helps.
>
> Thanks, it helps a great deal. I have more fanciful code to
> try. I've snagged an example from CGI Programming with Perl to
> model after but I suspect my if-elsing above looks like I'm
> still cgi-libbing.

Nobody will criticize your code if you have an explanation of
why each line was written. Patchwork software never works very
well, nor for very long.

HTH, as ever.

Rob




-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to