Hi Gabor,

On Sat, 5 May 2012 16:28:31 +0300
Gabor Szabo <[email protected]> wrote:

> On Fri, May 4, 2012 at 12:57 PM, Shlomi Fish <[email protected]> wrote:
> > Hi all,
> >
> > in reflection about Gabor’s talk yesterday, I’ve been spending some time
> > refactoring the code of the Config::IniFiles CPAN module, of which I am a
> > co-maintainer, and which I adopted because I’ve made use of it for one 
> > project.
> >
> > While doing that I’ve ran into this gem:
> >
> > sub GetFileName
> > {
> >    my $self = shift;
> >    my $filename;
> >    if (exists $self->{cf}) {
> >        $filename = $self->{cf};
> >    } else {
> >        undef $filename;
> >    }
> >    return $filename;
> > }
> >
> > It's indicative of a lot of ignorance of how Perl 5 works.
> >
> > This ended up being shortened into:
> >
> > sub GetFileName
> > {
> >    my $self = shift;
> >
> >    return $self->{cf};
> > }
> >
> 
> This is wonderful. When you see more such examples, please share with us.
> I think we can all learn from these!

I'll try to. Of course, by learning from these, one learns what not to do. 

> 
> > Anyway, later I've ran into this loop:
> >
> >                # process continuation lines, if any
> >                while($self->{allowcontinue} && $val =~ s/\\$//) {
> >                    $line = $self->_read_next_line($fh);
> >                    $val .= $line;
> >                }
> >
> > Now, $self->{allowcontinue} is not modified within _read_next_line() (or
> > anywhere else except for initialisation) so in case it is true, it is
> > evaluated every time the condition is processed (not to mention, it gave me 
> > a
> > "WTF?" moment).
> 
> This is a tricky one. There might have been some earlier version where the
> $self->{allowcontinue} was changed during the call and this might be a
> left over or
> maybe this is was a way  to avoid another level of indentation.

I doubt that $self->{allowcontinue} could ever change during the call, because
it's a configuration option given to the object at startup and it isn't
expected to change (except possibly by the user).

So this code seems to be too clever and also possibly slower than a version
with an if and a while inside it.   

> 
> I know I wrote such code and I am sure I still do even though this is
> probably some
> misguided optimization of the code.

OK. Well, I think that if the loop has many iterations, it will make matters
slightly worse, so this kind of thing should be avoided.

Regards,

        Shlomi Fish

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
"The Human Hacking Field Guide" - http://shlom.in/hhfg

In Soviet Russia, every time you kill a kitten, God masturbates.
    — http://linux.slashdot.org/comments.pl?sid=195378&cid=16009070

Please reply to list if it's a mailing list post - http://shlom.in/reply .
_______________________________________________
Perl mailing list
[email protected]
http://mail.perl.org.il/mailman/listinfo/perl

Reply via email to