My first thought on reading your node is that you somehow like to cause problems for yourself. I am forced to assume that you must not have read FamousLongAgo's link to Dominus' famous rebuttal of symbolic references. If you just missed it then go read that first. Your general approach right now is just painful and will be even more painful in the future when it breaks in strange and mysterious ways.

In general you don't have to copy all the parameter data off of the CGI object's param() list. You could just grab it directly wherever you need it. Your example number four is the best of the lot. If you didn't have that bit on "within this loop: also check for specific, or no data on each field" then you could reduce that to a subroutine. If you really feel somehow constrained from cherry-picking your expected results from param() then just use this.

 sub param_hash {
     my $q = shift;
     my %param;
     $param{$_} = $q->param($_) for $q->param;
     return \ %param
 }
[download]

Your examples had a variety of misunderstandings and errors. I've corrected them here. Consider this another reason why you personally shouldn't be using this feature right now. I feel like a broken record here but this feature requires some significant understanding and is not for use in everyday magic.

Example number one just highlights that you didn't predeclare your variables. That's just good code hygiene. Consider this like brushing your teeth - just do it.

Example two is flawed because now you've misunderstood the difference between pad variables and global variables. Symbolic references modify globals, not lexicals. You must change your my ($varA, $varB, $varC ....) (and those are particularly poor choices for variable names) lines to use vars qw($varA $varB $varC); or our ($varA, $varB, $varC). Once you've declared your globals then you can refer to them.

Example three is incoherent. I don't know why you're using an array - perhaps for notational convenience? Anyhow, unless you plan to some hash-ify your array keys you're exchanging something nice like $cgi_param{'sort_preference'} for $cgi_param[5] which is a hell of a lot less readable. You could have previously said use constant SORT_PREFERENCE => 5; in which case it'd actually be ok (and for a few reasons also good) to say $cgi_param[SORT_PREFERENCE]. Using constants as array indexes has the benefit of giving you some compile-time typo checking so you can't inadvertantly say $cgi_param[SRRT_PREFERENCE].

You also erred in sorting your parameter list - that's a but in your implementation. If you really planned on using an array like that then you'd probably say this instead:

 my @field_names = $q -> param;
 my @field_values = ();
 for my $index (0 .. $#field_names) {
     $field_values[$index] = $q->param( $field_names[$index] );
     # within this loop: also check for specific, or no data on each f
+ield
 }
[download]

At this point I hope it's clear to you that Best Practices indicate that you either (a) fetch the values as needed from your CGI object, (b) copy the values to a hash, or (c) copy the values to an array but use constants instead of direct array offsets. Actually best practices probably also state that you should pre-declare your expected form parameters and only operate off of those. That approach might look something like this:

 # Predeclare which parameters are needed and also specify
 # which are expected to contain lists. All others are
 # expected to contain scalars.
 use constant EXPECTED_PARAMETERS =>
     { sort_preference => '',
       signature       => '',
       display_fields  => 'ARRAY' };

 # Get the CGI object
 my $q = CGI->new;

 # Get the parameters and store in a hash reference
 my $params = get_parameters( $q, EXPECTED_PARAMETERS );

 # Do whatever the script does
 .....

 sub get_parameters {
     # The first parameter is the CGI object.
     # The second parameter is an hash reference
     # to the list of expected parameters. The values
     # are examined for the string 'ARRAY' and those
     # are then retrieved into array references.
     my $q = shift;
     my $expected = shift;

     # Get a list of all the parameter names in a hash
     # This allows me to do exists() tests for parameters
     my %params;
     my @params{ $q -> params } = ();

     # Now go get each expected parameter
     my %extracted;
     while (my ($param_name,
                $param_type) = each %$expected) {
         # warn of missing parameters
         unless (exists $params{$param_name}) {
             cluck "Missing CGI parameter $param_name";
             next;
         }
         
         # Copy the value over. This does a bit of fancy
         # footwork by extracting 'ARRAY' parameters into
         # arrays and all others into a plain scalar. It is
         # not sufficient to simply copy over values since
         # the behaviour for CGI's param() function changes
         # depending on the value of wantarray().
         $extracted{$param_name} =
             $param_type eq 'ARRAY'
             ? [ $q -> param($param_name) ]
             : $q -> param($param_name);

     # Return the expected parameter list as a
     # hash reference.
     return \ %extracted
 }
[download]

Fun Fun Fun in the Fluffy Chair

I thank you for the time you put into responding to my problem. The article you recomended certainly highlights a serious problem with using '$$variable'.

re: para2... I need the loop because I am testing and resetting the data depending on what is submitted, but thanks for the tip.

As to the rest of your comments, the code was there in an atttempt to highlight what I was trying to do. I know it's messy and I don't use variable names like those shown in those examples. I had preciously asked the question about declaring groups of variables but got no response. I had seen my ($firstvar, $secondvar, $thirdvar) used on this site and thought it was correct. I'll use our ($firstvar, $secondvar, $thirdvar) in future!

Example 3 is a messy attempt (that works) to get my $fieldname = $q->param('fieldname') for each field in the HTML doc while also checking and resetting the data within the loop. I didn't like it either.

Anyway, based on the responses above and your comments, I propose the use a hash as in example 4.

The hash is mostly ok except for the issue I raised with wantarray() in the code I posted at the bottom. You could of course just steal that sub and get something that Just Works. (not that I tested it or anything but it looks right)

---

Fun Fun Fun in the Fluffy Chair

Using a hash to store the variables could actually make the construction of the SQL statements easier and clearer. If you happen to be building an "insert into my_table ..." sort of statement, a hash makes this very simple to do via a "for" loop -- e.g.:

...
my $col_spec = my $val_spec = "(";
for my $column ( keys %data )
{
   $col_spec .= "$column,";
   $val_spec .= "$data{$column},";
}
$col_spec =~ s/,$/\)/;
$val_spec =~ s/,$/\)/;

my $sql = "insert into my_table $col_spec values $val_spec";
[download]

my $col_spec = join( ",", sort keys %data );
my $val_spec = join( ",", map { $data{$_} } sort keys %data );
my $sql = "insert into my_table ($col_spec) values ($val_spec)";
[download]

If the basic form of the sql statement will always involve the same set of columns and conditions, you should be using the DBI "prepare()" method with parameterized values (using the "?" placeholder, and passing the values as a list to the "execute()" method).

Also, remember that you can use any sort of whitespace as part of an SQL statement, including line-feeds and tabs, so even long constructs can be formatted so that they are not difficult to read.

update: as pointed out in the next reply below, the values being assembled should be quoted if you are not using the parameterized form of sql statement. (Thanks, and ++, blokhead!)

Always remember to quote data or else use placeholders! ;)

#### added call to $dbh->quote

my $col_spec = join ',', sort keys %data ;
my $val_spec = join ',',
               map  { $dbh->quote( $data{$_} ) }   ### here
               sort keys %data;
my $sql      = "insert into my_table ($col_spec) values ($val_spec)";

#### or with placeholders:

my $col_spec = join ',', sort keys %data;
my $val_spec = join ',', ('?') x keys %data;
my $sql      = "insert into my_table ($col_spec) values ($val_spec)";

$dbh->prepare($sql)
    ->execute( map { $data{$_} } sort keys %data );
[download]

Also it might be worth mentioning that the keys of %data should be validated before they are trusted in this code. Cheers.

blokhead

Actually, there's no real need to sort the keys. And there's definitely no need to sort them more than once: just create an array with them. A hash slice makes the map unnecessary. Here's a modified version of your code:

#### or with placeholders:
my $col_spec = join ',', keys %data;
my $val_spec = join ',', ('?') x keys %data;
my $sql      = "insert into my_table ($col_spec) values ($val_spec)";

$dbh->prepare($sql)->execute(@data{keys %data});
[download]

In case you want to use the perepared statement more than once, there's no ganratee that keys() will return the keys in the same order over different hashes even though they have the same keys:

#### or with placeholders:
my(@col_names, $sth);
{
    local $" = ',';
    @col_names   = keys %data;
    my $val_spec = join ',', ('?') x @col_names;
    my $sql      = "insert into my_table (@col_names) values ($val_spe
+c)";
    $sth = $dbh->prepare($sql);
}
[download]

And much later (just make sure $sth and @col_names are still in scope):

$sth->execute(@data{@col_names});
[download]

graff, Being relatively new to Perl, I do not fully understand your response, but I'll figure it out. It does indeed solve the underlying problem of improving the readability of the, later used, SQL statements. I'll also take note of blokhead's response.

Thank's to both of you. I'll stay with example 4 and use a Hash to gather the data.

You can't create global variables when using strict but you can create lexical variables. You do this with 'my'. See Coping with Scoping as well as other entries in the Tutorials section.

Update:BTW, do you have warnings turned on? $$fieldname = ... should have issued a warning with strict turned on.

---
print map { my ($m)=1<<hex($_)&11?' ':'';
$m.=substr('AHJPacehklnorstu',hex($_),1) }
split //,'2fde0abe76c36c914586c';
[download]

I don't think that will help do what the author intended. The problem isn't creating new variables, it's creating new ones with dynamic names (and then being able to use them under strict). Using lexicals doesn't solve this:

# trying to create a $dynamic_var

use strict;
my $var_name    = 'dynamic_var';

my $$var_name   = 'foo';         # doesn't compile
my ${$var_name} = 'foo';         # nor does this

eval "my \$$var_name = 'foo';";  # this runs fine, now $dynamic_var
                                 # exists in some scope

print $dynamic_var, "\n";        # but this doesn't work in strict
[download]

strict requires that variables be predefined before their use, whether global or lexical. However, when dynamically creating variables of any kind, this can't be the case. You may be able to bend things a bit to get the new variables created, but you can't actually use those new variables in the rest of your strict-enabled program (it sounds like nedals wants to use strict as much as possible in the remainder of the code). Please correct me if I'm missing some clever way of creating dynamic lexicals.

I think a hash is the best way to go.

blokhead

Thanks for the clarification blokhead. This is exactly the problem I had in mind.

Just want to expand a little bit on what pfaut said about $$fieldname.

First let's look at a piece of correct code:

use strict;
$$a = 1;# you cannot say my $$a ;-)
print $$a; #this print out 1;
print $a; #this print out SCALAR(0xnnnnnnnn)
[download]

This is perfectly correct, as I just created a hard reference, which is allowed. The next piece is wrong.

use strict("refs");
my $a = "abc";
my $abc = 1;
print $$a;
[download]

This is wrong, as I attempted to create symbolic reference, which is disallowed when you have strict("refs") turned. As a best practice, symbolic reference should be generally disallowed, as it is the source of many bugs in Perl scripts.

You can't create global variables when using strict but you can create lexical variables

This isn't strictly true (NPI). You can't use variables that either haven't been declared (see. my|/our and vars) or aren't fully qualified while the strict 'vars' pragma is in use e.g

shell> perl -c -
use strict 'vars';
use vars qw( $foo );

$foo       = "package variable declared with use vars";
my $bar    = "lexical variable";
$main::baz = "fully qualified package variable";
our $quux  = "YAWTDAV";

$::abc     = 'shortcut for $main::abc';
${*{$main::{x}}{SCALAR}} = 'verbosely assign to $::x';

- syntax OK
[download]


HTH

_________
broquaint

my %data = $q->Vars;
[download]

use CGI ':standard';
my $q = new CGI;
$q->import_names('Input'); # Replace 'Input' with any name

print <<"DONE";
Content-type: text/html\n
<html><body>
  Your Name: $Input::name<br>
  Your Text: $Input::text
</body></html>
DONE
[download]

I suggest you keep using strict and use a hash!!!

You could build your SQL statements differently...

$SQL='select * from table where :name=:value';
$SQL=~s/:(\w+)/$data{$1}/g;
[download]

a better alternative for fixed fields (you are using DBI, right??)

$sth = $dbh->prepare('select * from table where name=?');
$rv=$sth->execute($data{name});
[download]

I.E.
my $var1 = "FOO"; #Now its declared.
my $var2;

#Now you can do whatever you want to it.  

$var2 = "BAR";
$var1 = $var1 . " " . $var2;
[download]

     %data = %param;
[download]