bladx has asked for the wisdom of the Perl Monks concerning the following question:
Hi everyone. Is there anyway I can really improve the following small piece of code for a little program that deals with CGI for the internet, where you have a web page that has say 3 links on it leading to other relative pages on the site. When you click on a link you want it to take you to another page and stuff, and it uses the GET command and stuff from the cool CGI.pm parser. In short, I was just wondering how I could improve this code to make it as efficient as I can. It's a really simple little CGI program in Perl, but it's just for a future project of mine, and I would like to know if it needs to be changed around a bit or a lot.
#!/usr/bin/perl -wT
use strict;
use CGI ':all';
use CGI::Carp qw(fatalsToBrowser);
if (param('page') eq "") {
&index;
}
elsif (param('page') eq "page2") {
&page2;
}
elsif (param('page') eq "page3") {
&page3;
}
sub index {
print header(),
start_html('INDEX'),
'<p>This is the index page!<br><br>',
'<a href=index.pl?page=>index</a><br>',
'<a href=index.pl?page=page2>page2</a><br>',
'<a href=index.pl?page=page3>page3</a><br>',
end_html();
}
sub page2 {
print header(),
start_html('PAGE_2'),
'<p>This is page2!<br><br>',
'<a href=index.pl?page=>index</a><br>',
'<a href=index.pl?page=page2>page2</a><br>',
'<a href=index.pl?page=page3>page3</a><br>',
end_html();
}
sub page3 {
print header(),
start_html('PAGE_3'),
'<p>This is page3!<br><br>',
'<a href=index.pl?page=>index</a><br>',
'<a href=index.pl?page=page2>page2</a><br>',
'<a href=index.pl?page=page3>page3</a><br>',
end_html();
}
Thanks for any help in advance! Bye now.
bladx ~ a seeker of many perl questions
Edit: 2001-03-03 by neshura
Re: Improving the efficiency of this piece of code
by chipmunk (Parson) on Feb 25, 2001 at 06:16 UTC
|
I would probably use a hash for this, with the values as subroutine references. Here's a simple example:
#!/usr/bin/perl -wT
use CGI qw(:all);
use CGI::Carp qw(fatalsToBrowser);
my %dispatch = ( '' => \&index,
page2 => \&page2,
page3 => \&page3,
default => \&index,
);
my $page = param('page');
my $subref = $dispatch{$page} || $dispatch{default};
$subref->();
| [reply] [d/l] |
|
| [reply] |
|
I can personally vouch for CGI::Application's homemade goodness. I found it a tad easier to learn than HTML::Mason while it provides similar functionality. Basically, it provides runtime function callbacks on various invocations of the script with some shortcuts in between. I've find this module killer if combined with HTML::Template (or some other templating mechanism).
One drawback is that the entire generated HTML must be passed back as a string when you're done- hence the web l0ser will not see any stuff until the script is absolutely done- ala zero NPH capability (or similar capability). This is especially frustrating when used in congruence with DBI. But, as the doctor says when you ask "Doctor, I'm fat in some place!", simply don't use massive amounts of CGI-generated HTML with CGI::Application.
AgentM Systems nor Nasca Enterprises nor
Bone::Easy nor Macperl is responsible for the
comments made by
AgentM. Remember, you can build any logical system with NOR.
| [reply] |
|
| [reply] |
Re (tilly) 1: Improving the efficiency of this piece of code
by tilly (Archbishop) on Feb 25, 2001 at 06:14 UTC
|
First of all please don't play games with the font. The
usual one is fine.
Secondly rather than calling functions using:
&index;
it is better to call it with
index();
so you don't accidentally pass in the wrong parameters.
(The first form reuses your current parameters.)
Third I would suggest that you have a final case that
reports what the parameter was and that it was not
understood. This will help debugging when (in my experience
not if) things go sour.
Fourth in chatter you got the suggestion to call
param('page') once and put that in a variable.
That idea is a good one.
Fifth, you are using strict, good, but I think you are
using too much from CGI. Try :standard until that runs
into problems. (Or using the OO form.)
I could add more, but this is probably more than enough to
digest at one go... | [reply] [d/l] [select] |
|
I've made a habit of calling functions as &FUNC(); to make them more readily visible.
The following explanations would seem to approve my approach. Are there other factors I should take into account? Reason(s) why I should forego the preceeding "&"? Particular situations, perhaps?
perlman::perlsub tells me:
"& is optional with parentheses" and that "& makes current @_ visible to called subroutine".
perlman::perlvar says:
"within a subroutine the array @_ contains the parameters passed to that subroutine. See the perlsub manpage".
thanks,
Don
striving for Perl Adept
(it's pronounced "why-bick")
Update: After reading tye's post that Albannach linked below, and re-reading tilly's post above, I *think* I'm OK in continuing to &FUNC().
| [reply] |
|
| [reply] |
Re: Improving (the efficiency of) this piece of code
by dvergin (Monsignor) on Feb 25, 2001 at 06:16 UTC
|
bladx
In asking for help making this code more efficient, you
imply that this code runs. It does not. Let's work on that
first.
First, you have inadvertantly picked a subroutine name that
conflicts with with a built-in Perl function. 'index'
is a perl function and so Perl cannot see your 'index'
subroutine. Let's name it index_page.
Next, your test:
if (param('page') eq "")
coughs up an error if param('page') does not
exist as is the case after the index_page()
subroutine. So we reword that as:
if ( ! defined(param('page')) )
Now your script looks like this:
#!/usr/bin/perl -wT
use strict;
use CGI ':all';
use CGI::Carp qw(fatalsToBrowser);
if ( ! defined(param('page')) ) {
index_page();
}
elsif (param('page') eq "page2") {
page2();
}
elsif (param('page') eq "page3") {
page3();
}
sub index_page {
print header(),
start_html('INDEX'),
'<p>This is the index page!<br><br>',
'<a href=index.pl?page=>index</a><br>',
'<a href=index.pl?page=page2>page2</a><br>',
'<a href=index.pl?page=page3>page3</a><br>',
end_html();
}
sub page2 {
print header(),
start_html('PAGE_2'),
'<p>This is page2!<br><br>',
'<a href=index.pl?page=>index</a><br>',
'<a href=index.pl?page=page2>page2</a><br>',
'<a href=index.pl?page=page3>page3</a><br>',
end_html();
}
sub page3 {
print header(),
start_html('PAGE_3'),
'<p>This is page3!<br><br>',
'<a href=index.pl?page=>index</a><br>',
'<a href=index.pl?page=page2>page2</a><br>',
'<a href=index.pl?page=page3>page3</a><br>',
end_html();
}
Which runs and happily bounces back and forth to the
different virtual pages when you hit the various links
in your browser.
Hope this helps. I don't see any obvious 'efficiency' issues
here.
Upward and onward. Keep at it. You have good, clean
presentation but you did not test this code before posting
it. If you had, your question would have likely been
something like:
"Why do I get...
Not enough arguments for index at
C:\DocsMisc\PerlCode\index.pl line 8... etc. etc.'
...when I run this script?"
The things I have mentioned here, I learned by reading the
error messages your code produced, fixing the error, running
again, fixing the next thing, etc. Error messages are your
friends. Very helpful little beasties!
Cheers,
dv | [reply] [d/l] [select] |
Re: Improving the efficiency of this piece of code
by unixwzrd (Beadle) on Feb 25, 2001 at 06:03 UTC
|
One thing off the top of my head would be to do a:
my $page = param('page');
before the condition tests. That way you only make one call
to param.
Mike - mps@discomsys.com
"The two most common elements in the universe are hydrogen... and stupidity."
Harlan Ellison
| [reply] [d/l] |
(ichimunki) re: Improving the efficiency of this piece of code
by ichimunki (Priest) on Feb 25, 2001 at 18:23 UTC
|
Two comments on your hard-coded HTML:
1. Stop It. You have already used CGI and imported all its methods? Use them. The functional approach is very easy to use, and will make this code a lot easier to read. Also, it is infinitely better to rely on a computer to properly close tags and other mechanical stuff, than it is to have to remember to do that stuff yourself. I know, I've wasted countless hours mucking around with simple HTML issues because it's hard to see what's what when it's scattered all over a perl script.
2. While some older standard of HTML made it possible to omit the quotes from your tag attributes, doing so just makes it that much harder to read. As a further note on #1, if you use the functional CGI approach for more than the headers, the module will take care of this for you. | [reply] |
Re: Improving the efficiency of this piece of code
by grinder (Bishop) on Feb 26, 2001 at 13:42 UTC
|
A general point of order: don't print a list of strings,
print just one multi-line string:
print header(),
start_html('INDEX'),
q{<p>This is the index page!<br><br>
<a href="index.pl">index</a><br>
<a href="index.pl?page=page2">page2</a><br>
<a href="index.pl?page=page3">page3</a><br>},
end_html();
We're not in BASIC any more.
| [reply] [d/l] |
Re: Improving the efficiency of this piece of code (slightly OT, somewhat similar approach)
by ybiC (Prior) on Feb 25, 2001 at 19:02 UTC
|
I don't have an answer to your question, bladx, but here's 2 ways that I've done something similar:
cheers,
Don
striving for Perl Adept
(it's pronounced "why-bick") | [reply] |
(Ovid) Re: Improving the efficiency of this piece of code
by Ovid (Cardinal) on Feb 25, 2001 at 22:22 UTC
|
| [reply] |
|
I don't believe that the overhead is in exporting the functions, but in the ability for the methods to "swing both ways".
Every dual-nature (ie. functional and OO) method in CGI starts off by calling an internal sub, self_or_default, to determine the calling style. If the method was called as an export, self_or_default has to do the additional task of unshifting the default CGI object ($Q in the CGI.pm source) onto @_. This negates any performance gain from not having the method-call overhead:
use strict;
use warnings;
use Benchmark qw(cmpthese);
use CGI qw(:standard);
my $q = new CGI;
cmpthese (-3, {
method => sub { $q->header; },
direct => sub { header; }
});
### RESULTS ####
Rate direct method
direct 14483/s -- -4%
method 15106/s 4% --
So it appears as if OO calling style on CGI is indeed faster in practice.
MeowChow
s aamecha.s a..a\u$&owag.print | [reply] [d/l] [select] |
Re: Improving the efficiency of this piece of code
by bladx (Chaplain) on Feb 25, 2001 at 05:58 UTC
|
sorry about those amp;'s i don't know why they are in there... it really should look like &index; etc... those are just calls to the subs.
bladx ~ ¡mucho veces yo tengo preguntas! | [reply] |
|
|