Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

use Perl Log In

Log In

[ Create a new account ]

Ovid (2709)

Ovid
  (email not shown publicly)
http://publius-ovidius.livejournal.com/
AOL IM: ovidperl (Add Buddy, Send Message)

Stuff with the Perl Foundation. A couple of patches in the Perl core. A few CPAN modules. That about sums it up.

Journal of Ovid (2709)

Wednesday September 13, 2006
03:34 AM

Object Responsibilities

[ #30979 ]

Me know nuthin' 'bout objects.

There are days when I feel really, really stupid. Today's stupidity has actually been going on for several weeks, simmering in the back of my mind. I just received a bug report for TAPx::Parser from Torsten Schönfeld. Seems that my parser doesn't like blank lines in the output. After some debugging, it became clear that the following is a big bucket of suck:

sub next {
    my $self = shift;
    if (@{$self->_m_tokens}) {
        return shift @{$self->_m_tokens};
    }
    if ($self->_current_chunk) {
        if ( $self->_stream_started ) {
            $self->_start_tap(0);
        }
        else {
            $self->_start_tap(1);
            $self->_stream_started(1);
        }
    }
    my $next_chunk = $self->_stream->next;
    if (! $self->_current_chunk() && $next_chunk ) {
        $self->_current_chunk($next_chunk);
        return $self->next;
    }
    unless ( defined $next_chunk ) {
        $self->_end_tap(1);
    }
    if ( defined $self->_current_chunk ) {
        my @current_tokens = map {
            my $result = TAPx::Parser::Results->new($_);
            $self->_validate($result);
            $result;
        } $self->_lex($self->_current_chunk());
        my $token = shift @current_tokens;
        push @{$self->_m_tokens()} => @current_tokens;
        $self->_current_chunk($next_chunk);
        return $token;
    }
    $self->_finish;
    return;
}

What the hell is that? I can barely follow it and I wrote it! I hated writing it at the time and I hate it even worse, now. But that's because I was really, really stupid and let a known design flaw remain in my parser. I didn't have my responsibilities appropriately distributed.

When I said at the beginning "me know nuthin'", though this is a bit of a slam on me, it's also sound advice about objects. I shouldn't know a damn thing about 'em. I should beg them for information and tell them what they want to know. That's it. I shouldn't know anything about their internals or, for that matter, their state. However, in the above code, I was actually trying to track the state of an iterator or stream. I wasn't reaching in, I was trying to track it externally. That was stupid.

If you really, really need to know the state of something and you can't ask it, you've probably got a design flaw. In the iterator code, I copied it almost verbatim from Test::Harness::Iterator. I'm sure that the Test::Harness code is fine for its needs, but it's completely innapropriate for mine. In fact, my subconscious was even trying to tell me I was being a moron because prior to that method you find the following comment:

# all of this annoying current and next chunk stuff is to ensure that we
# really do know if we're at the beginning or end of a stream.

And that horror is all because I didn't implement first and last methods for my iterator. Well, I've now finished rewriting the iterator, but it's on my computer at home and I'm at work, so I won't be able to upload right away. While I haven't actually implemented it, a first pass at rewriting the next method would look something like this:

sub next {
    my $self = shift;
    my $stream = $self->_stream;
    my $next = $stream->next;
    $self->_start_tap( $stream->first ? 1 : 0 );
    if ( $stream->last ) {
        $self->_finish;
        return;
    }

    # this is a tiny oversimplification
    my $token = TAPx::Parser::Results->new( $self->_lex($next) );
    $self->_validate($token);
    return $token;
}

That's much cleaner. If I had gone ahead and fixed this problem when I first knew about it, I wouldn't have had two serious bug reports about this method in the space of five days.

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.