- use
strict or die (updated)
- declare your variables with my
- Try CGI's builtin functions for generating the
header, form, tags and actually all html.
use CGI qw/:html3/
- I wouldn't use eval to trap errors. There is
an option for CGI like use CGI::Carp 'fatalsToBrowser'; (updated).
- The while loop could be done more concise:
while( <GUESTS> ){
#no chomp, we need the \n later on...
split /::/;
if( $_[0] =~ /$search_name) {
print join "<BR>\n", @_;
print "<HR>\n";
}
}
- Try use diagnostics;, maybe you like it. I do.
Jeroen
</code>
"We are not alone"(FZ)
| [reply] [Watch: Dir/Any] [d/l] [select] |
$name =~ /\Q$search_name\E/i
To make sure that special characters in your CGI parameter
don't accidentally do something you didn't intend. See
quotemeta and perlre.
buckaduck | [reply] [Watch: Dir/Any] [d/l] |
Quickly, why mix up subroutines and large chunks of code?
May I suggest something like:
&sub_one;
&sub_two;
&sub_three;
sub sub_one{
}
sub sub_two{
}
sub sub_three{
}
I think that this is clearer and easier to understand. It maximises the benefit of using subroutines.
Other than that I agree with the above comments on CGI and strict. CGI is very important from an upkeep point of view - so long as you keep your CGI module up to date you don't have to worry about rewriting your code to match the latest standards. strict is a very useful learning tool.
"The significant problems we face in life can not be solved at the same level of thinking we were at when we created them." -Albert Einstein | [reply] [Watch: Dir/Any] [d/l] |
Unless you have a specific reason to use the implicit argument list, it is best to use parens in calling functions.
If this comment made no sense to you, the following summary may help.
Also functions should be given names that clearly describe what they do. Inability to find a meaningful name for a function is usually an excellent indicator that the code is not well factored. Certainly having a function whose name does not properly describe what it does is almost always a maintainance problem...
| [reply] [Watch: Dir/Any] |
Above comments withstanding, here it goes:
Change this:
if ($query->param()) {
$search_name = $query->param('search_name');
...
into:
if( $search_name = $query->param('search_name') ) {
...
If the param('search_name') is empty/fails, it will return a blank string, '', which perl treats as a false value, i.e., if condition fails. Same effect as before, a little clearer.
Why are you doing <BR> and \n here? \n won't have any real effects on the html(usually), so it's not needed. Unless, I presume, you need to edit it, and this is for 'beautification purposes'. If you really want/need to line breaks, try <p>. This is where I'm talking about:
print "$name<BR>\n$email<BR>\n$comments<BR>\n<HR>\n";
Major Recommendation: Look up CGI.pm's HTML handling abilities! Especially its abilities with forms. It'll make your pages a lot less error prone, and the CGI a lot more bearable/easier to read for larger examples. For example, <title>this is the title</title> can be re-written, assuming $q is a cgi object, as $q->title("This is the title"). CGI will automatically add an /title, or end whatever tag, as needed. Look into it. | [reply] [Watch: Dir/Any] [d/l] [select] |
This was my post(I guess I got logged out somehow). Just thought I'd clear up any confusion.
| [reply] [Watch: Dir/Any] |