Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

review the current usage of each #109

Closed
stdweird opened this issue Sep 12, 2015 · 2 comments · Fixed by #277
Closed

review the current usage of each #109

stdweird opened this issue Sep 12, 2015 · 2 comments · Fixed by #277
Milestone

Comments

@stdweird
Copy link
Member

each is evil, and altough it can be used correctly, should be avoided at all costs. after learning the lesson once more (i ran into it previously, but somehow forgot), we should get rid of it.

the problem is that each sort-of remembers the last element it iterated over.
this has all sorts of nasty side effects, but the following example illustrates the danger of using each and stopping the iteration.

#!/usr/bin/perl

use strict;
use warnings;

my %h = (
    a => 1,
    b => 2,
    );

sub bad
{
    my $stop = shift;
    while (my ($k, $v) = each(%h)) {
        print "$k $v\n";
        return if $stop;
    }
}

sub good
{
    my $stop = shift;
    foreach my $k (keys%h) {
        print "$k $h{$k}\n";
        return if $stop;
    }
}


print "bad\n";
bad(1);
bad();

print "good\n";
good(1);
good();

which gives output

bad
b 2
a 1
good
b 2
b 2
a 1
@jrha
Copy link
Member

jrha commented Aug 23, 2023

AFAICT each is only being used in:

  • TextRender.pm
  • ReporterMany.pm

@jrha jrha added this to the 23.9 milestone Aug 23, 2023
@jrha jrha modified the milestones: 23.9, 23.next Sep 28, 2023
@jrha jrha modified the milestones: 25.next, 24.next Sep 4, 2024
@ned21
Copy link
Contributor

ned21 commented Sep 5, 2024

Should we add a section on this to https://www.quattor.org/development/coding_style.html ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants