next if loop

tux242 has asked for the wisdom of the Perl Monks concerning the following question:

Replies are listed 'Best First'.
Re: next if loop by Abigail-II (Bishop) on Nov 19, 2003 at 21:15 UTC
Yikes, what an awful mess. First of all, the indentation is a mess, making it harder to see what's going on. `foreach my $file (<passwd.*>){ open (PASSWD,"$file");` [download] Why quote $file? But most of all, you aren't checking the return value of your open. It might fail! `$nf="file1"; open (NEWFILE, ">$nf");` [download] Why do you do this in the loop? Now you repeatedly open the file, while you only want to do it once. And you aren't checking the return value of the open. `while (<PASSWD>) { ($login, $passwd, $uid, $gid, $gcos, $home, $shell) = split(/:/); $USERS{$login} = $gcos; } close (PASSWD); } foreach $login (sort keys %USERS) { $gcos = $USERS{$login}; @SKIP = ('adrian','adm','sys','alcatel'); foreach $login ($SKIP) { next if $login eq $SKIP; }` [download] Multiple problems here. First, $SKIP isn't defined (had you used strict or warnings, Perl would have told you). You want @SKIP in the 'foreach' line. Second, you are reusing $login. In the inner loop there's no way to refer to the outer $login. Third, you want to restart the outer loop, so you need a next with a label - now you just do the inner loop again. You could have written this is: `@SKIP = ('adrian','adm','sys','alcatel'); delete @USERS {@SKIP}; foreach $login (sort keys %USERS) { ... }` [download] `#next if ($login =~ /adrian\|adm\|sys\|alcatel/);` [download] Now, had you placed anchors (^ and $), this should have worked. `print NEWFILE "$login\n"; } close (NEWFILE);` [download] You should check the return value of your close. It might fail. Abigail	[reply] [d/l] [select]
Re: Re: next if loop by hardburn (Abbot) on Nov 20, 2003 at 19:08 UTC
You should check the return value of your close. It might fail. What's the point? If the filehandle isn't going to close, there's not much you can do about it, except maybe complain about it to STDERR. I suppose you could try calling `close` again and pray that it will work this time, but if it failed the first time, it will probably fail again. Is there really any use to catching errors from `close`? ---- I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident. -- Schemer `: () { :\|:& };:` Note: All code is untested, unless otherwise stated	[reply] [d/l] [select]
Re: next if loop by Abigail-II (Bishop) on Nov 20, 2003 at 19:48 UTC
I rather have a program that says "close failed: no space left on device" than a posting to perlmonks that say "I print to a file and it doesn't work". But that's just my personal preference. Abigail	[reply]
Re: Re: Re: next if loop by davido (Cardinal) on Nov 20, 2003 at 19:15 UTC
Is there really any use to catching errors from close? I believe there is, when the file was opened for output. The output buffer may not be flushed until the close. If the close fails, it could be indicative of a failure to flush the buffer, and thus, the file output may be incomplete or otherwise corrupted. This can happen due to a number of reasons. Someone may have taken the floppy out of the drive prematurely, the HD may have filled up, the CD may not have burned properly, the dialup connection (on an open socket) may have terminated early, etc. In such an event, I would want to know about it, rather than have a silent failure. My habit is to always put the '`or die "$!\n";`' after closing an output file. I don't worry too much about the success of closing an input file. Dave "If I had my life to live over again, I'd be a plumber." -- Albert Einstein	[reply] [d/l]
Re: next if loop by hardburn (Abbot) on Nov 19, 2003 at 21:12 UTC
`@SKIP = ('adrian','adm','sys','alcatel'); foreach $login ($SKIP) { next if $login eq $SKIP; }` [download] You're not iteratating over the array `@SKIP`, but the scalar `$SKIP` (which I don't see defined in your code). These are two completely seperate values, though they share a typeglob. `use strict` would have caught this error, unless `$SKIP` is defined elsewhere in your code (which I think you meant to do, since that's what you're comparing `$login` to). You could also do what you want with: `@SKIP = grep { $_ ne $SKIP } 'adrian','adm','sys','alcatel';` [download] ---- I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident. -- Schemer `: () { :\|:& };:` Note: All code is untested, unless otherwise stated	[reply] [d/l] [select]
Re: next if loop by liz (Monsignor) on Nov 19, 2003 at 21:06 UTC
`@SKIP = ('adrian','adm','sys','alcatel'); foreach $login ($SKIP) { next if $login eq $SKIP; }` [download] What do you think this does? You're setting up an array `@SKIP` and then you use the scalar `$SKIP` to loop on? And then in the loop, you're checking `$SKIP` again. `use strict;` is your friend. Use it. Liz Update: Removed reference to $_, thanks to hardburn for pointing this out!	[reply] [d/l] [select]
Re: Re: next if loop by hardburn (Abbot) on Nov 19, 2003 at 21:16 UTC
whereas you should be checking $_ What good would that do? The `foreach` loop is saving the data to `$login`, not `$_`. ---- I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident. -- Schemer `: () { :\|:& };:` Note: All code is untested, unless otherwise stated	[reply] [d/l] [select]
Re: next if loop by Zaxo (Archbishop) on Nov 20, 2003 at 06:15 UTC
I'd like to recommend getpwnam for this. Here we get a HoA keyed on user names, containing all the user info for each listed username. `my %users = map { $_ => [ getpwnam $_ ] } qw( adrian adm sys alcatel );` [download] That eliminates all the fuss with the passwd file, to the benefit of portability. After Compline, Zaxo	[reply] [d/l]