All the Perl that's Practical to Extract and Report

Happiness is...

cog on 2008-07-24T18:01:10

...when you notice you just did with 23 lines of code what the previous programmer did with 425.

sub parse { my $self = shift;

my $feed = XML::Feed->parse( \$self->get_content() );

my @items;

for my $entry ( $feed->entries ) { push @items, { title => $entry->title, link => $entry->link, description => $entry->content->body || $entry->summary->body, date => $entry->issued || $entry->modified, author => $entry->author, }, }

return { feed_title => $feed->title, items => \@items, }; }

That over there is parsing Atom, RSS1, and RSS2 just the way the system needs it to be done.

And yes, the project where those 425 lines could be found was the one I mentioned a few days ago.

(apparently the code tags are messing up the allignment of the => and ||)

Or just over half that :)

educated_foo on 2008-07-24T22:39:34

sub parse {
    my $feed = XML::Feed->parse(\shift->get_content);
    { feed_title => $feed->title,
      items => [
          map {
         {title => $_->title,
          link => $_->link,
          description => $_->content->body || $_->summary->body,
          date => $_->issued || $_->modified,
          author => $_->author }
     } $feed->entries]}
}

# Incidentally, why "get_content", but "content" and "body"?

Re:Or just over half that :)

jarich on 2008-07-25T02:02:16

ew! Implicit return? You wouldn't have added an extra line by adding in those 6 letters; but you would have improved readability a fair bit. With what you've written I have to skim to the end to see what you're returning and then work my way upwards to see where the last statement starts. That might be okay when golfing, but that doesn't appear to be your intent. Damian spends almost 3 whole pages on the evils of implicit returns in "Perl Best Practices". See 197-199.

Otherwise, well down on shrinking the code even further. ;) I'd have used a little more whitespace than you, but I know that getting formatting to work out exactly right can be a challenge.

Re:Or just over half that :)

educated_foo on 2008-07-25T14:07:43

I tend to write in an expression-oriented style, e.g. "map" instead of "foreach", and implicit return fits right in. I also try to limit vertical whitespace in order to fit more stuff on the screen. Both are just a matter of taste, though.

I haven't read PBP, but from what I've heard about it, the "best practice" seems to be to ignore it. It has a few bits of good advice, but most seems arbitrary or just plain wrong.

Re:Or just over half that :)

Aristotle on 2008-07-25T14:31:57

You shouldn’t judge a book by its critics.

Almost everyone who passes comment on the book seems to have read right past the first chapter where Damian explains that what’s important is not to follow his mandates, but to read the arguments and think about them. The book’s goal is not to get you to write code the way Damian tells you to, but to get people to consciously reflect on their own tastes and proclivities to analyse the impacts of their own choices, particularly as they relate to each other.

Re:Or just over half that :)

educated_foo on 2008-07-26T01:49:29

I'm actually judging by what I've read of it online (see e.g. other comment). And the statement you mention is eminently reasonable. The problem is that people seem to interpret "PBP" as "the Best Perl Practices," not "My Perl Practices," and to follow them slavishly.

Re:Or just over half that :)

jarich on 2008-07-25T23:55:10

As you haven't read PBP, I recommend you do so. Even if you then go and ignore most of the content, I'm sure you'll still learn a few things you didn't know before and become a better coder because of it. I'm dubious about it being a common recommendation from other experienced Perl programmers that the best practice is to ignore it, but I guess we know different people.

I have no issue with you using map instead of foreach, and often use it myself, but I don't agree with leaving off the return. As a Perl trainer, I can assure you that this will completely baffle many less experienced coders. Some will assume your subroutine returns nothing, others will spend a lot longer than I did spotting where the last statement begins. But even with those arguments aside, relying on implicit returns can make your code wrong.

For example (from Damian's book):

sub find_second_odd {
    my $prev_odd_found = 0;

    # Check through args
    for my $num (@_) {
        # Find an odd number
        if(odd($num)) {
            # Return it if it's not the first
            return $num if $prev_odd_found;

            # Otherwise, remember we've seen it
            $prev_odd_found = 1;
        }
    }
    # Otherwise we've failed
}

I know this is a contrived example. Providing good, easy to understand examples of problems which often bite you in much more complex systems is hard.

So what's going wrong in that subroutine?

What does this return for the following cases (also from Damian's book):

find_second_odd(2..6);
find_second_odd(2..1);
find_second_odd(2..4);
find_sec ond_odd(2..3);

Hmm, that's right. 5, undef, empty string, empty string.

Huh?

Damian explains what's going on, on page 198. ;)

Re:Or just over half that :)

educated_foo on 2008-07-26T01:47:36

As you haven't read PBP, I recommend you do so. Even if you then go and ignore most of the content, I'm sure you'll still learn a few things you didn't know before and become a better coder because of it. I'm dubious about it being a common recommendation from other experienced Perl programmers that the best practice is to ignore it,

That's not what I said, or at least not what I meant to say. Let's take a look at this: 1) okay; 2) okay if you use TDD, irrelevant otherwise; 3) a good way to end up with pointless boilerplate documentation; 4) good idea; 5) okay; 6) meh, enforcing human readability with a program is a recipe for failure; 7) uselessly vague; 8) okay; 9) again, useful iff TDD; 10) good advice that's usually given and/or taken the wrong way.

So we've got 4 good ideas, 3 neutral ones, and 3 bad ones. Most of the good ones seem obvious, so I'm not inclined to buy the book.

I have no issue with you using map instead of foreach, and often use it myself, but I don't agree with leaving off the return. As a Perl trainer, I can assure you that this will completely baffle many less experienced coders.

That's interesting to know, but luckily, I don't have to care if they understand my code.

For example (from Damian's book): [...] I know this is a contrived example. Providing good, easy to understand examples of problems which often bite you in much more complex systems is hard.

So what's going wrong in that subroutine?

What's the last expression in that sub?

lets not forget

link on 2008-07-25T00:19:14

the other 25000 lines of code XML::Feed loads. Modules rock and all but sometimes I wonder should it take me 20 minutes to install something because it has a bazillion dependencies. Each of those is something bad waiting to happen.

Re:lets not forget

ChrisDolan on 2008-07-25T02:57:54

Are you optimizing for machine performance or developer performance? In my experience, the latter is way more important these days, and CPAN is the best accelerant in the Perl world.

And, your point about bugs in other people's modules is sometimes valid, but in general I've found that other people write better code in their area of expertise than I write outside of my expertise.

RSS/Atom is one of those areas where expertise *really* matters. Have you ever seen some of the test cases Sam Ruby has for the Python feed validator? Yikes!