On Jun 11, David vd Geer Inhuur tbv IPlib said:

>open(FH, "< $groupfile");
>@usrs = <FH>;
>close FH;
>
> $htusr = (grep {/htuser: /} @usrs)[0] ;
> $phi = (grep {/phil: /} @usrs)[0] ;
> $al = (grep {/all: /} @usrs)[0] ;

That looks wasteful to me.  You're looping over the data FOUR[1] times,
instead of just getting the data as you come across it in the file:

  open FH, "< $groupFile";
  while (<FH>) {
    last if $htuser and $phil and $all;
    $htuser = $_, next if /htuser: /;
    $phil = $_, next if /phil: /;
    $all = $_, next if /all: /;
  }
  close FH;

Now we only go through the data ONE time.

> @htuser = split(/ /, $htusr);
> @phil = split(/ /, $phi);
> @all = split(/ /, $al);

Why not do this in the loop above?

  open FH, "< $groupFile";
  while (<FH>) {
    last if @htuser and @phil and @all;
    @htuser = split, next if /htuser: /;
    @phil = split, next if /phil: /;
    @all = split, next if /all: /;
  }
  close FH;

> foreach $htuser(@htuser) {
>  chomp $htuser;
>  if ( "$pwuser" eq "$htuser" ) { $group = "iclab"; }
> }

This is ok, except a) useless use of chomp(), and b) useless use of quotes
around variables.  But even moreso, you should probably use a hash instead
of an array for this.

  my %usergroup;
  my %groupname = (
    htuser => 'iclab',
    phil => 'phil',
    all => 'all',
  );    

  open FH, "< $groupFile";
  while (<FH>) {
    my ($grp, $members) = split /:\s*/;
    $usergroup{$_} = $groupname{$grp} for split ' ', $members;
  }
  close FH;

This changes your problem to something MUCH simpler.  Now you have a hash,
%usergroup, that you can search for the key matching the username $pwuser:

  my $group = $usergroup{$pwuser} || "none";

See?

But I ask another question -- why can't you use the getpwnam() function
for this?

  my $gid = (getpwnam $pwuser)[3];
  my $group = getgrnam $gid;

  # or

  my $group = getgrnam +(getpwnam $pwuser)[3];

Unless you're devising your OWN group names, this is a much simpler (and
better) way to do things.


[1] once to read the file, three times in the grep()s

-- 
Jeff "japhy" Pinyan      [EMAIL PROTECTED]      http://www.pobox.com/~japhy/
RPI Acacia brother #734   http://www.perlmonks.org/   http://www.cpan.org/
** Look for "Regular Expressions in Perl" published by Manning, in 2002 **
<stu> what does y/// stand for?  <tenderpuss> why, yansliterate of course.
[  I'm looking for programming work.  If you like my work, let me know.  ]


-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to