texasperl has asked for the wisdom of the Perl Monks concerning the following question:

Good (morning|afternoon|evening) Monks! My question, as always, relates to the 'best' way of doing things. I am constantly striving to improve my Perl, and with that, I provide to you the following (working) code that will generate an ARIN SWIP reassign template based on form input. It's rough, but I'm looking for places to improve. Any and all suggestions are greatly appreciated! Without further ado:
#!/usr/bin/perl -w use strict; use CGI; use CGI::Carp qw( fatalsToBrowser ); # not leaving in production! # this is hackish and ugly using n_ for keys, but sort() likes it my %swip_data = ( "02_org_name" => "", "03_org_addr1" => "", "03_org_addr2" => "", "04_org_city" => "", "05_org_st" => "", "06_org_zip" => "", "07_org_cc" => "", "09_poc_type" => "", "10_poc_role" => "", "12_poc_cname" => "", "13_poc_addr1" => "", "13_poc_addr2" => "", "14_poc_city" => "", "15_poc_st" => "", "16_poc_zip" => "", "17_poc_cc" => "", "18_poc_phone" => "", "19_poc_email" => "", "20_ip_addr" => "", "21_netname" => "", ); my $cgi = new CGI; print $cgi->header; print $cgi->start_html(-title => "ARIN SWIP Generator"); print "Welcome to the ARIN Shared WHOIS Project Template Filler!<br>"; print "In development...pretty straightforward although your mileage m +ay vary.<br>Please check results for sanity!"; print "<br><br>"; my @sorted = sort keys %swip_data; # loop through our ordered keys and match fields to template values print $cgi->start_form(); foreach (@sorted) { print "$_ :" . $cgi->textfield(-name => "$_") . "<br>"; } foreach (@sorted) { if (defined $cgi->param("$_")) { $swip_data{$_} = $cgi->param("$_"); } else { print "You must define a $_." . "<br>"; } } print $cgi->submit(-value => 'Black Magic'); print $cgi->end_form(); my $arin_tmpl = <<END_TMPL Template: ARIN-REASSIGN-DETAILED-4.0<br> ** As of September 2006<br> ** Detailed instructions are located below the template<br>. 01. Downstream Org ID:<br> 02. Org Name: $swip_data{'02_org_name'}<br> 03. Org Address: $swip_data{'03_org_addr1'}<br> 03. Org Address: $swip_data{'03_org_addr2'}<br> 04. Org City: $swip_data{'04_org_city'}<br> 05. Org State/Province: $swip_data{'05_org_st'}<br> 06. Org Postal Code: $swip_data{'06_org_zip'}<br> 07. Org Country Code: $swip_data{'07_org_cc'}<br> 08. Org POC Handle:<br> 09. Org POC Contact Type (P or R): $swip_data{'09_poc_type'}<br> 10. Org POC Last Name or Role Account: $swip_data{'10_poc_role'}<br> 11. Org POC First Name: <br> 12. Org POC Company Name: $swip_data{'12_poc_cname'}<br> 13. Org POC Address: $swip_data{'13_poc_addr1'}<br> 13. Org POC Address: $swip_data{'13_poc_addr2'}<br> 14. Org POC City: $swip_data{'14_poc_city'}<br> 15. Org POC State/Province: $swip_data{'15_poc_st'}<br> 16. Org POC Postal Code: $swip_data{'16_poc_zip'}<br> 17. Org POC Country Code: $swip_data{'17_poc_cc'}<br> 18. Org POC Office Phone Number: $swip_data{'18_poc_phone'}<br> 19. Org POC E-mail Address: $swip_data{'19_poc_email'}<br> ** NETWORK SECTION<br> 20. IP Address and Prefix or Range: $swip_data{'20_ip_addr'}<br> 21. Network Name: $swip_data{'21_netname'}<br> END_TMPL ; print $arin_tmpl; print $cgi->end_html();
Many thanks in advance.

Replies are listed 'Best First'.
Re: How to loop through CGI params and insert into a heredoc?
by fenLisesi (Priest) on Oct 20, 2006 at 17:52 UTC
    use strict; use warnings; my %swip_data = map { $_ => q() } qw( 02_org_name 03_org_addr1 03_org_addr2 04_org_city 05_org_st 06_org_zip 07_org_cc 09_poc_type 10_poc_role 12_poc_cname 13_poc_addr1 13_poc_addr2 14_poc_city 15_poc_st 16_poc_zip 17_poc_cc 18_poc_phone 19_poc_email 20_ip_addr 21_netname ); for my $field (sort keys %swip_data) { printf qq(%-12s => "%s"\n), $field, $swip_data{ $field }; }
    prints: Update: For the form stuff you may want to see CGI::FormBuilder or Data::FormValidator.
Re: How to loop through CGI params and insert into a heredoc?
by shmem (Chancellor) on Oct 20, 2006 at 21:40 UTC
    Looks fine to me so far. I'd just reverse populating hash and array:
    my @sorted = qw( 02_org_name 03_org_addr1 03_org_addr2 04_org_city ... ); my %swip_data; @swip_data{@sorted} = map { defined $cgi->param($_) and $cgi->param($_) or print "You must define a $_.<br>" and '' } @sorted;
    Done. No need to sort, hash populated, missing fields reported. Oh. wait...

    <update>... you are printing the form fields first:

    my @sorted = qw( 02_org_name 03_org_addr1 03_org_addr2 04_org_city ... ); my %swip_data; my @missing; @swip_data{@sorted} = map { print "$_ :" . $cgi->textfield(-name => "$_") . "<br>"; defined $cgi->param($_) and $cgi->param($_) or push @missing, $_ and ''; } @sorted; @missing and print "You must define these fields: ",join(', ', @missin +g),".<br>\n";

    No need for double quotes here:

    if (defined $cgi->param("$_")) {

    and no need for explicit concatenation here:

    print "You must define a $_." . "<br>"; print "You must define a $_.<br>"; # less noise

    Unless you use $arin_tmpl elsewhere, just say

    print <<END_TMPL;

    If you populate the array first, and from there your hash, you don't need n_ to maintain order.

    </update>

    --shmem

    _($_=" "x(1<<5)."?\n".q·/)Oo.  G°\        /
                                  /\_¯/(q    /
    ----------------------------  \__(m.====·.(_("always off the crowd"))."·
    ");sub _{s./.($e="'Itrs `mnsgdq Gdbj O`qkdq")=~y/"-y/#-z/;$e.e && print}