chad kellerman wrote:
> Hello everyone,
>
> I want to clean this bit of code up. It looks really messy. I am in a
> mental block. Any suggestions?
>
> @userInfo = split /\s+/, $buffer;
>
> #insert home users & quota in the db
> foreach $userInfo ( @userInfo ) {
>
> ( $name, $quota ) = split /\|/, $userInfo;
> # get rig of the header info from repquota
Right here is where you can start. Once you trim the user name, lowercase it
lc($name);
that will provide case insensitivity, presuming that was your intent. This should
obviate the need to use such an unwieldy regex. I would recommend storing that list
of authorized users in a file, reading it into an array, and then looping through that
array, using the eq operator.
Beyond aesthetics, ugly and clunky structures have a very practical ill effect: they
make code harder to track, and thus increase the likelihood of error.
>
>
> next if ( ( $name =~ /Block/ ) or ( $name =~ /nobody/ ) or ( $name =~
> /User/ ) or ( $name =~ /^\d/ ) or ( $name =~ /www/ ) or ( $name =~ /backup/
> ) or ( $name =~ /ftp/ ) or ( $name =~ /httpd/ ) or ( $name =~ /root/ ) or (
> $name =~ /netop/ ) or ( $name =~ /sysop/ ) or ( $name =~ /users/ ) or (
> $quota !~ /\d+/ ) or ( $name =~ /^#/ ) or ( $quota <= 8 ) or ($name =~
> /bill/) );
Rob already gave you the best suggestion for regex alternatives, the '|' operator.
Another thing that will help you keep your code neat is using a code editor. there
are good code editors avaiable for both Winodws and 'nix. If you use Windows, I
recommend Programmer's File Editor, available free at
http://www.lancs.ac.uk/people/cpaap/pfe/.
You need to get a better understanding of indentation conventions. Programming
languages use indentation to mark execution blocks. Statements that are executed in
sequence should be kept aligned. When a block, whether a subroutine definition, a
conditional, a loop, or any other structures that sets off a set of lines for separate
execution sequence, those lines affected should be indented by a specific [your taste,
but preferably no more than four] number of *spaces* [tabs don;t translate well].
for (1..10) {
do_one_thing();
then_another;
if ($some_condition_exists) {
take_first_response_step();
follow_up_with_response();
} else {
take_alternate_course();
}
}
Notice something? With absolutely no coded specifics, you know how this program is
going to execute, where the effect of any control statement starts and stops, and what
the overall flow of the logic will be. Good, consistent indentation habits will make
your work much easier.
Let's take another crack at your problem here:
my @names = qw (
Block nobody User www backup ftp httpd root
netop sysop users
)
foreach $userInfo ( @userInfo ) {
my ($name, $quota ) = split /\|/, $userInfo;
# get rig of the header info from repquota
my $matched;
foreach (@names) {
if lc($_) eq lc($name) {
matched = 1;
last;
}
}
if ($match or ($name =~ /^\d/) or ( $quota !~ /\d+/ ) or
( $name =~ /^#/ ) or ( $quota <= 8 ) or ($name =~ /bill/)) {
next;
}
}
Note that I pulled out the or'ed conditions that were not really logically parralel.
Having them mixed in that way can be a bit dangerous to comprehension. Is there any
reason you had the conditions for $quota interleaved with those for $name in this way?
Joseph
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
