Hi Richard, First of all you are right this is more than a little inefficient. I would advise even more modifications though.
1. Separate the business logic and user interface parts in separate modules. 2. Use templates for the displaying and don't have the program create the user interface. 3. Send the kid that build this back to school. As to how to do this well you are on the right path, by passing a set of variables to the sub you will be able to determine what to draw. Basically your subs seem to have only two lines that distinguish them, the hash name and the table header. A variable or two should be able to handle that. Think of somehting like this (when separating the display and business logic is to much work): sub generic { my $Header = shift || 'Unknown catagory'; my $Hash = shift || return undef; < the rest of the sub upto the print header bit > print "<p align=center><b><font color=$config{'colortablebody'} face=Arial size=2>$Header</font></b></td></tr>"; < a bit more of the code upto the key decliration > foreach $key (sort keys %{$Hash} ) { < the remaining code > } Call the sub in the following way: generic( "The heading we desire", \%Washers ); Of course %Washers is just a random hash you could use any of them there... BUT DO NOT FORGET THE \ ;-) On 3/29/06, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: > > I'm really new to Perl and am learning as I go. My boss gave me a > program to look over that they're thinking of implementing. It seems to > be a GPL auction program. As I was looking through the code, I was > puzzled by one part... the part that displays subcategories on the users > screen. I've copied snippets of the code below. Specifically - the "sub" > is called by the line of code in the top block - I.E. the "washers" sub > is called to display the "washers" subcategory. What sort of seems very > inefficient is if I want to add twenty more categories I have to > duplicate and add (with minor changes) 20 more "subs". This will really > result in program bloat if we have many subcategories... Isn't it > possible to somehow only have ONE "sub" and pass parameters to it in the > call to it that can make a "generic" sub work for ALL the subcategories > we want to use? If so, any suggestions on how I can do this? (I'm aware > that the solution may be obvious to Perl experts, but sadly not to me > yet.... but it seems like in other languages this is possible...but how > in Perl?). Thanks!! > > The calling lines followed by the "subs"... > > > ======================================================================== > ========== > > elsif ($form{'action'} eq 'washers') { &washers; } #washers > Category > elsif ($form{'action'} eq 'bolts') { &bolts; } #bolts > Category > elsif ($form{'action'} eq 'cotterpins') { &cotterpins; } #cotterpins > Items Category > > ======================================================================== > ========== > > > > sub washers { > &chkclose; > print $config{'subheader'}; > print "<div align=center><center>"; > print "<table border=0 cellspacing=0 style=border-collapse: > collapse width=50% bgcolor=$config{'colortablehead'}>"; > print "<tr><td width=100%>"; > print "<div align=center><center>"; > print "<table border=0 cellpadding=2 cellspacing=0 > style=border-collapse:collapse bordercolor=$config{'colortablehead'} > width=100%>"; > print "<tr><td width=100% colspan=2 > bgcolor=$config{'colortablehead'}>"; > print "<p align=center><b><font color=$config{'colortablebody'} > face=Arial size=2>Washers Categories</font></b></td></tr>"; > my $key; > foreach $key (sort keys %washers) { > umask(000); # UNIX file permission junk > mkdir("$config{'basepath'}$key", 0777) unless (-d > "$config{'basepath'}$key"); > opendir DIR, "$config{'basepath'}$key" or &oops("Category > directory $key could not be opened."); > my $numfiles = scalar(grep -T, map "$config{'basepath'}$key/$_", > readdir DIR); > closedir DIR; > print "<tr><td width=87% bgcolor=$config{'colortablebody'}>"; > print "<p align=left><font face=Arial><small><A > HREF=$ENV{'SCRIPT_NAME'}\?category=$key\&listtype=current>$category{$key > }</A></td>"; > print "<td width=13% bgcolor=$config{'colortablebody'}><font > SIZE=2 face=Arial><p > align=center><b>\($numfiles\)</b></font></td></tr>"; > } > print "</table></center></div></td></tr></table></center></div>\n"; > } > > > > sub bolts { > &chkclose; > print $config{'subheader'}; > print "<div align=center><center>"; > print "<table border=0 cellspacing=0 style=border-collapse: > collapse width=50% bgcolor=$config{'colortablehead'}>"; > print "<tr><td width=100%>"; > print "<div align=center><center>"; > print "<table border=0 cellpadding=2 cellspacing=0 > style=border-collapse:collapse bordercolor=$config{'colortablehead'} > width=100%>"; > print "<tr><td width=100% colspan=2 > bgcolor=$config{'colortablehead'}>"; > print "<p align=center><b><font color=$config{'colortablebody'} > face=Arial size=2>bolts Categories</font></b></td></tr>"; > my $key; > foreach $key (sort keys %bolts) { > umask(000); # UNIX file permission junk > mkdir("$config{'basepath'}$key", 0777) unless (-d > "$config{'basepath'}$key"); > opendir DIR, "$config{'basepath'}$key" or &oops("Category > directory $key could not be opened."); > my $numfiles = scalar(grep -T, map "$config{'basepath'}$key/$_", > readdir DIR); > closedir DIR; > print "<tr><td width=87% bgcolor=$config{'colortablebody'}>"; > print "<p align=left><font face=Arial><small><A > HREF=$ENV{'SCRIPT_NAME'}\?category=$key\&listtype=current>$category{$key > }</A></td>"; > print "<td width=13% bgcolor=$config{'colortablebody'}><font > SIZE=2 face=Arial><p > align=center><b>\($numfiles\)</b></font></td></tr>"; > } > print "</table></center></div></td></tr></table></center></div>\n"; > } > > > > sub Cotterpins { > &chkclose; > print $config{'subheader'}; > print "<div align=center><center>"; > print "<table border=0 cellspacing=0 style=border-collapse: > collapse width=50% bgcolor=$config{'colortablehead'}>"; > print "<tr><td width=100%>"; > print "<div align=center><center>"; > print "<table border=0 cellpadding=2 cellspacing=0 > style=border-collapse:collapse bordercolor=$config{'colortablehead'} > width=100%>"; > print "<tr><td width=100% colspan=2 > bgcolor=$config{'colortablehead'}>"; > print "<p align=center><b><font color=$config{'colortablebody'} > face=Arial size=2>Cotterpins Items Categories</font></b></td></tr>"; > my $key; > foreach $key (sort keys %Cotterpins) { > umask(000); # UNIX file permission junk > mkdir("$config{'basepath'}$key", 0777) unless (-d > "$config{'basepath'}$key"); > opendir DIR, "$config{'basepath'}$key" or &oops("Category > directory $key could not be opened."); > my $numfiles = scalar(grep -T, map "$config{'basepath'}$key/$_", > readdir DIR); > closedir DIR; > print "<tr><td width=87% bgcolor=$config{'colortablebody'}>"; > print "<p align=left><font face=Arial><small><A > HREF=$ENV{'SCRIPT_NAME'}\?category=$key\&listtype=current>$category{$key > }</A></td>"; > print "<td width=13% bgcolor=$config{'colortablebody'}><font > SIZE=2 face=Arial><p > align=center><b>\($numfiles\)</b></font></td></tr>"; > } > print "</table></center></div></td></tr></table></center></div>\n"; > } > > > > > Portions of this message may be confidential under an exemption to Ohio's > public records law or under a legal privilege. If you have received this > message in error or due to an unauthorized transmission or interception, > please delete all copies from your system without disclosing, copying, or > transmitting this message. > >