|
Re: How to write better code?
by Corion (Patriarch) on Dec 04, 2006 at 15:01 UTC
|
You want to use the CGI.pm module, which can parse that:
use strict;
use CGI;
my $string = "name=alex&sex=m&age=20";
my $q=CGI->new($string);
print Dumper scalar $q->Vars()"
If you actually are, as it looks, parsing a CGI query, the CGI module can also take all of the other chores of parsing CGI queries for you:
use strict;
use CGI;
my $q=CGI->new(); # parses all CGI parameters
print $q->header('text/plain');
print Dumper scalar $q->Vars()"
| [reply]
[d/l]
[select] |
Re: How to write better code?
by jbert (Priest) on Dec 04, 2006 at 15:17 UTC
|
To elaborate on what the others have said: one aspect of good coding/experience is to appropriately choose library code rather than write everything yourself. If you are, as seems likely, parsing CGI parameters, then either the well-used CGI.pm or a related module (e.g. CGI::Lite) might be of use. As you might know, you'll find lots of information on modules on CPAN. This is one of perl's major strengths - there is a lot of useful code already written. (But you do need to be a little selective - perhaps some of it doesn't quite do what you need, etc.)
In the specific case of the code you've posted, one of the best pieces of advice I'd give is to name things (variables, functions, objects, methods) carefully - and don't be afraid of renaming them if their purpose (or your understanding of their purpose changes).
In this example, people are guessing that you're writing CGI-parsing code, because you've got variables called $string and @array. This doesn't really convey any information to the reader. It would be much better to write something like:
my $cgi_query_string = "name=alex&sex=m";
my @components = split(/&/, $cgi_query_string);
my %query;
foreach my $component (@components) {
my ($name, $value) = split(/=/, $component);
$query{$name} = $value;
}
otherwise, what you've written is fairly good, idiomatic perl. If you're interested in some other, nice, more modern idioms, take a look in List::Util and List::MoreUtils. These don't really apply to your code above, but they are a nice way of removing some loops and lines from your code and simultaneously expressing the intent of what you're trying to do more clearly.
Also, there is a lot (too much?) in the [id://Tutorials] section and also a lot of good advice in the book "Perl Best Practices". | [reply]
[d/l]
[select] |
|
|
otherwise, what you've written is fairly good, idiomatic perl
It's pretty decent perl but it's lousy cgi param parsing:
- no handling of multivalued params
- no handling of ; as separator
- no handling of url encoded params or values
| [reply] |
|
|
I agree with other comments regarding the advisability of use CGI; if parsing CGI. I also agree with your advice to the OP about meaningful variable names. However, while the code does what it is meant to do, I think it might be more readable if the foreach loop and the intermediate variables are dispensed with
my $cgi_query_string = "name=alex&sex=m";
my %query =
map {split m{=}}
split m{&}, $cgi_query_string;
I realise this might be a step too far for some but I find it easier to see the wood for fewer trees. Others may (and probably will) disagree.
Cheers, JohnGG | [reply]
[d/l]
[select] |
|
|
It's an interesting point. I almost put in a paragraph on this (saying that some people would suggest dropping the naming variables).
I tend to view naming vars almost a substitute for comments. They can be a way of better documenting your intent. I mused about that a bit here.
That said, there's also a lot to be said for brevity, and I like your code too :-).
If I'm honest, I suspect I'd have dispensed with the @components intermediate array, but written out the foreach. I think this is because I rarely think of assigning arrays to lists (and avoiding using a side-effect like $query{$x} = $y in a map clause is one of my reasons for using an explicit foreach). Maybe I should do that more often.
One hint is that I had a little trouble coming up with a good name for the intermediate array - always a good sign that it probably isn't necesary/desirable.
| [reply]
[d/l]
[select] |
|
|
| [reply] |
|
|
Re: How to write better code?
by SheridanCat (Pilgrim) on Dec 04, 2006 at 16:38 UTC
|
I agree with all the other comments. Just one note on using Data::Dumper. If you pass it a reference, you'll get more readable output. Your code produces:
$VAR1 = 'name';
$VAR2 = 'alex';
$VAR3 = 'age';
$VAR4 = '20';
$VAR5 = 'sex';
$VAR6 = 'm';
If you just passed Dumper() a reference, like this:
print Dumper \%hash_init;
You'll get this:
$VAR1 = {
'name' => 'alex',
'age' => '20',
'sex' => 'm'
};
Which is a whole lot nicer to look at. | [reply]
[d/l]
[select] |
Re: How to write better code?
by McDarren (Abbot) on Dec 04, 2006 at 15:07 UTC
|
use CGI qw( Vars );
use Data::Dumper;
my %params = Vars()
print Dumper(\%params);
Cheers,
Darren :) | [reply]
[d/l] |
Re: How to write better code?
by fenLisesi (Priest) on Dec 04, 2006 at 15:02 UTC
|
Depending on where the $string comes from, the answer might be use CGI; Cheers. | [reply]
[d/l]
[select] |
Re: How to write better code?
by artist (Parson) on Dec 07, 2006 at 21:05 UTC
|
Assuming your string comes from a CGI query, following code is the shortest and works best for your use. It will display nicely in HTML format.
use CGI;
print CGI->new->Dump;
I would suggest that use read 'Perl Best Practices' book.
| [reply]
[d/l] |
