[EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:
Please don't top post.
: First of all thank you very for your help. Following my
: Code. Do you know where the fault is?
You just had to ask that, didn't you?
: #!/usr/bin/perl
:
: if ($0 =~ /(.*)\/[^\/]*$/) { push (@INC, "$1"); }
: if ($0 =~ /(.*)\\[^\\]*$/) { push (@INC, "$1"); }
Statement modifiers tend to be easier to read. Don't
quote variables.
push @INC, $1 if $0 =~ /(.*)\/[^\/]*$/;
push @INC, $1 if $0 =~ /(.*)\\[^\\]*$/;
Are you sure you don't want to 'unshift' those?
Shouldn't you be checking for trailing duplicate entries
in @INC? Look at lib.pm.
: use site_lowlevel;
Why use an eight space indent for the 'if' statements
and switch to a two space indent for the 'use' statement.
Indent code blocks uniformly. Be consistent.
: }
Either this isn't the complete code or that closing
brace shouldn't be there.
: use site_lib;
: use site_dlib;
: use site_config;
: use site_iolib;
: use File::Find;
: use site_form_lib;
: use site_createlib;
Always use strict and warnings.
use strict;
use warnings;
: %META_META = ();
: @search_fields = ();
Don't use ALL CAPS for variables without a stated reason.
Don't declare variables before they are used. Declare them
just before or on their first use.
: $Block_Count = 0; # A 'Block' is until the next
: # CLEARLIST so each READDIR only
: $TIME = time; # CAPS means current time! c.k.
Avoid end comments. Especially on scripts sent via email.
They often wrap in the messages and they look ugly. Some of
your variables use underscores to separate words. This one
uses mixed case along with underscores. Other variable names
do not follow this pattern. Find one style and stick to it.
Be consistent. Read perlstyle.
# A 'Block' is until the next CLEARLIST so each READDIR only
my $block_count = 0;
The comment doesn't make sense in this context. What is a
CLEARLIST? and what READDIR is being referred to? Comments
are meant to relieve confusion, not to aid it.
my $TIME = time(); # CAPS means current time! c.k.
That is not a good reason to use ALL CAPS. Why explain
in the comment that $TIME is the current time? Use this --
no explanation needed.
my $current_time = time();
: &active_finddate ($TIME);
Read perlsub. Don't call subroutines using the & prefix.
Why separate the first word in the subroutine name and not
the second? Be consistent.
my $current_time = time();
active_find_date( $current_time );
Or just:
active_find_date( time() );
: $YEAR = $year;
: $FULLYEAR = $fullyear;
To be consistent that should be $FULL_YEAR and
$full_year.
: $MON = $mon;
: $MONAT = $monat;
What is a "monat"? Why not use descriptive words?
: $MDAY = $mday;
: $DAYOFWEEK = $dayofweek;
$DAY_OF_WEEK and $day_of_week;
: $SEC = $sec;
$seconds
: $HOUR = $hour;
: $MIN = $min;
$minutes
The lowercase variables have not been defined. We have
to assume that they were defined in active_find_date(), but
a good programmer would have then passed those variables
back (read perlsub). Here we use a hash slice to do the
same thing, but now we know where the values came from.
my %current;
@current{ qw(
two_digit_year
four_digit_year
month
monat
day
day_of_week
seconds
hour
minutes ) } = active_find_date( time() );
We can use $current{two_digit_year} instead of
$year. No comments needed. Read perlfunc 'return'.
: $savepage_string = ""; # if you want to make
The comment isn't helping. There are three words
in the variable name.
my $save_page_string = '';
: $taker_count = -1;
:
: $basepath = "$SYSTEM_CONF{'DOCUMENT-ROOT'}";
: $path = "$SYSTEM_CONF{'SITE-DIR'}";
Don't quote variables.
my $base_path = $SYSTEM_CONF{DOCUMENT-ROOT};
my $path = $SYSTEM_CONF{SITE-DIR};
: %FORM = DecodeFormData (); # formulare auswerten
Use CGI.pm to decode form data. Avoid using a hash to
hold field values. Here we have yet another naming convention
(mixed case). Choose one style. Be consistent.
: @cookie_fields = split (/\,/, $ENV{'HTTP_COOKIE'});
: &split_cookie_fields; # Cookies auswerten
Use CGI.pm (or a cookie module) for cookies.
: $checkfile = "indigo.htms"; # do not display this file
How does the comment relate to the statement?
my $check_file = 'indigo.htms';
: print "Content-Type: text/html\n";
As others have mentioned.
print "Content-Type: text/html\n\n";
Or:
use CGI 'header';
print header();
: $level = "";
: undef %YET_DONE;
You can't undef a hash that hasn't been defined anywhere.
If it was defined in a subroutine, it should remain scoped to
that subroutine.
HTH,
Charles K. Clarkson
--
Mobile Homes Specialist
254 968-8328
_______________________________________________
ActivePerl mailing list
[EMAIL PROTECTED]
To unsubscribe: http://listserv.ActiveState.com/mailman/mysubs
