Re: Get those parameters without CGI.pm

[coolmichael]

I've been thinking about trying to do this for a while now. It would be an interesting challenge to try and write something like CGI.pm. It would be nice to make it smaller, possibly chop it into smaller pieces. I hope you understand that I'm not going to attack you for trying to write a replacement.

You wrote:

Well, instead of just telling me it's bad and doesn't solve anything, how about telling me what I could do to improve it? I doubt you wrote wonderful, perfictly optimised code shortly after you started learning perl.

Ok here goes.

1. It doesn't pass use strict. In order to make that happen, you need to declare these varriables:
   $cookie, %cookie, %params, $form, %form
2. In the body of the function you store the cookie information in %cookies but you return %cookie. I really don't think that's right at all. use strict; would have caught that one.
3. $one and $two are bad varriable names. It would be more readable if you had used $cookiename and $cookievalue, but that is more of a style thing.
4. It is a bad idea to store name=value pairs in cookies, but a lot of websites do just that. I don't think your cookie code will work on that. For example (which is an actual google cookie) this isn't the correct result.

   Cookie:ID=7b6dfe0a3ce82015:FF=4:LD=en:NR=20:TM=1023854170:LM=1023854194:S=wEgMDFiEqMG
   one:ID
   two:7b6dfe0a3ce72015:FF

5. I don't understand why you need to put your cookie data in to your %params. That just duplicates data needlessly. It also has a bug as you mentioned, that form data will override your cookie data in the %params. Why not just avoid the situation all together and keep %cookies, %params, and %form seperate?
6. Ovid is quite right. You are using the same regular expression on four lines. This is a bad thing. It should be a function.

   s/%([\da-fA-F][\dA-Fa-f])/pack("C", hex($1))/eg;

7. Other monks have mentioned that you don't properly handle ALL valid query strings (ie ?foo=bla&foo=lsi will ignore the second foo parameter)
8. Merlyn is right, your regular expression to decode the hex strings is slow.
9. s/\+/" "/eg isn't wrong, it's just not the best way to do it. If you insist on using s/// then try s/\+/ /g; (you don't need the /e modifier and you don't need quotes around your space.) a faster way still would be to use the tr operator I think. You don't need if $one =~ m/\+/. Also, you've got that same s/\+/" "/eg regular expressioin in four places. That would be much better off as a seperate subroutine. You'll see why if you change all four of them.
10. return %cookies, %form, %params; doesn't do what you probably think it does. It returns one list, not three. In perl you can not pass or return more than one list. for example, with this query string

    http://localhost/cgi-bin/japh.cgi?abc=123&foo=bar&foo=were&blar&cose

    print "Content-Type: text/plain\n\n&";
    my (%cookie, %form, %params) = get_params();
    print Dumper \(%cookie, %form, %params)

    output the following:

    $VAR1 = {
          'foo' => 'were',
          'cose' => undef,
          'abc' => '123',
          'blar' => undef,
          };
    $VAR2 = {};
    $VAR3 = {};
    
          

That is ten things that I see needing to be changed or fixed. Number 10 is the biggie. Once you have parsed your query string and filled %form and %params you don't get them returned the way you seem to be expecting. Also, because of the way that perl combines the hashes into lists, you can't tell ahead of time which keys will over ride which. I would suggest that you return references to your hashes, to keep them seperate.