Beefy Boxes and Bandwidth Generously Provided by pair Networks
good chemistry is complicated,
and a little bit messy -LW
 
PerlMonks  

Improving the efficiency of this piece of code

by bladx (Chaplain)
on Feb 25, 2001 at 05:27 UTC ( #60708=perlquestion: print w/ replies, xml ) Need Help??
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

Comment on Improving the efficiency of this piece of code
Download Code
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!
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

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...

      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().

        Coincidentally I ran across this node today in which tye offers a nice summary of calling options.

        --
        I'd like to be able to assign to an luser

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

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->();
      I really like this way of doing things. I implement it myself in a lot of my CGI code too. It seems cleaner to me than all the condition testing, and though I haven't benchmarked it, I would assume faster using the hash to dispatch.

      Mike - mps@discomsys.com

      "The two most common elements in the universe are hydrogen... and stupidity."
      Harlan Ellison

      I'd also consider communing with the almighty vault. It seems that someone else actually has created a multiscreen CGI before....

      CGI::Screen and CGI::Application are the ones I've noticed there. But I haven't actually used them, so I don't know what, (?:dis)?advantages they have compared to a simple hash dispatch method.

        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.
(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.
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
(Ovid) Re: Improving the efficiency of this piece of code
by Ovid (Cardinal) on Feb 25, 2001 at 22:22 UTC
    One thing I'd like to hear comment about: I've read that CGI.pm has to jump through so many hoops to export its functions that the OO interface is actually as fast OR FASTER than the functional interface. If anyone has more info about that, I'd love to hear more.

    Cheers,
    Ovid

    Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

      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
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.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://60708]
Approved by root
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others browsing the Monastery: (10)
As of 2014-09-02 13:14 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite cookbook is:










    Results (22 votes), past polls