Re: How to write better code?

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;
}
[download]

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

Comment on Re: How to write better code? Select or Download Code

Replies are listed 'Best First'.
Re^2: How to write better code? by derby (Abbot) on Dec 04, 2006 at 15:46 UTC
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 -derby	[reply]
Re^2: How to write better code? by johngg (Canon) on Dec 04, 2006 at 16:58 UTC
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;` [download] 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]
Re^3: How to write better code? by jbert (Priest) on Dec 04, 2006 at 17:18 UTC
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]
Re^3: How to write better code? by imp (Priest) on Dec 04, 2006 at 17:20 UTC
Your implementation using map instead of foreach is slightly more readable, but will be more cumbersome to maintain and debug if additional parsing is required. It is likely a short-term win that will result in a comment block that is larger than the code after a few upgrades are made. Another weakness of using map for this is that CGI allows a parameter to have multiple values, but the code you provided will only use the last value.	[reply]
Re^4: How to write better code? by johngg (Canon) on Dec 04, 2006 at 20:21 UTC
If the task becomes more complex then I would almost certainly rewrite the code with more explicit control structures and descriptive variable names rather than jump through hoops making the `map` fit. I am only throwing away one line, when all's said and done. I agree with your comment about CGI and said as much at the top of my post, which was more a comment on the code snippet itself and it's legibility rather than it's suitability for parsing CGI. Cheers, JohnGG	[reply] [d/l]