Tag: refactoring

Posted on
by

Introduction: The current state of play

Perl has ​“very minimal” support for object-​oriented (OO) programming out of the box by its own admission. It’s class-​based but classes are just packages used differently. Objects are just data structures blessed into a class, methods are just subroutines whose first argument is an object or class name, and attributes/​properties are often just the key-​value pair of a hash stored in the object. (This last is a feature shared with JavaScript, whose prototype-​based objects are just collections of key-​value pairs with the keys addressed as properties.) You’ve got polymorphism, inheritance, and it’s up to you to enforce encapsulation.

This can take a lot of work to use effectively. To help address that, several systems have been developed over the years to reduce boilerplate and provide modern (or ​“postmodern”) OO features that developers from other languages expect. My favorite for a while has been Moo: it’s got the features I need 90% of the time like built-​in constructors, roles (an alternative to composition through inheritance), attributes, type validation, and method modifiers for enhanced polymorphism. And if I need to dig around in the guts of classes, attributes, and the like I can always upgrade to Moo’s big brother Moose and its meta-​object protocol with minimal effort.

Corinna, Object::Pad, and porting dbcritic

But there’s a new kid on the block. Curtis ​“Ovid” Poe has been spearheading Corinna, an effort ​“to bring effective OO to the Perl core and leapfrog [emphasis his] the capabilities of many OO languages today.” No CPAN modules, no chain of dependencies; just solid OO features and syntax built-​in. And while Corinna is a ways off from shipping, Paul ​“LeoNerd” Evans (maybe I should get a cool nickname too?) has been implementing some of these ideas as new Perl keyword syntax in his Object::Pad module.

Both Ovid and LeoNerd have been asking developers to try out Object::Pad, not just as a new toy, but to get feedback on what works and what needs to be added. So I thought I’d try porting an older small Moo-​based project named dbcritic to this new reality. In the process, I learned some of the advantages and disadvantages of working with Object::Pad. Hopefully, this can inform both it and Corinna’s evolution as well as other curious developers’ evaluations. You can follow my coding efforts in this GitHub branch.

First, the marquee result: the code for App::DBCritic (the class I started with) is cleaner and shorter, with 33 lines shaved off so far. Mainly this is due to Object::Pad’s more concise attribute syntax (called ​“slots” in its documentation) and lack of explicit support for Moo’s attribute coercion. I only used the latter for one attribute in the Moo version and I’m not sure it worked particularly well, so it wasn’t hard to jettison. But if your code supports coercions extensively, you’ll have to look into Object::Pad’s BUILD or ADJUST phase blocks for now.

Before, a Moo attribute with various options:

has schema => (
    is        => 'ro',
    coerce    => 1,
    lazy      => 1,
    default   => \&_build_schema,
    coerce    => \&_coerce_schema,
    predicate => 1,
);

After, an Object::Pad slot. No coercion and builder code is handled in a later ADJUST block:

has $schema :reader :param = undef;

Speaking of ADJUST blocks, it took a little bit of insight from the #perl IRC channel to realize that they were the appropriate place for setting slot defaults that are computed from other slots. Previously I was using a maze of dependencies mixing Moo lazy attributes and builder methods. Clarifying the main set of optional constructor arguments into a single ADJUST block helped untangle things, so this might be an indication that lazy attributes are an antipattern when trying to write clean code. It’s also worth noting that Object::Pad ADJUST blocks run on object construction, whereas Moo lazy attributes are only built when needed. This tends to matter for database access.

The ADJUST block for the $schema slot:

ADJUST {
    my @connect_info = ( $dsn, $username, $password );
    if ($class_name and eval "require $class_name") {
        $schema = $class_name->connect(@connect_info);
    }
    elsif ( not ( blessed($schema) and $schema->isa('DBIx::Class::Schema') ) ) {
        local $SIG{__WARN__} = sub {
            if ( $_[0] !~ / has no primary key at /ms ) {
                print {*STDERR} $_[0];
            }
        };
        $schema = App::DBCritic::Loader->connect(@connect_info);
    }
    croak 'No schema defined' if not $schema;
}

Object::Pad’s slots have one great advantage over Moo and Moose attributes: they directly support Perl array and hash data structures, while the latter only supports scalars and references contained in scalars. This means methods in your class can eliminate a dereferencing step, again leading to cleaner code. I used this specifically in the @violations array and %elements hash slots and was very pleased with the results.

The @violations and %elements slots and their ADJUST blocks:

has %elements;

ADJUST {
    %elements = (
        Schema       => [$schema],
        ResultSource => [ map { $schema->source($_) } $schema->sources ],
        ResultSet    => [ map { $schema->resultset($_) } $schema->sources ],
    );
}

has @violations;

ADJUST {
    @violations = map { $self->_policy_loop( $_, $elements{$_} ) }
        keys %elements;
}

method violations { wantarray ? @violations : \@violations }

Issues

I did have some development lifecycle issues with Object::Pad, but they’re mainly a result of its future-​facing syntax. I had to give up using perltidy and perlcritic in my build and test phases, respectively: perltidy doesn’t understand slot attributes like :reader and :param and will emit an error file (but code still compiles), and several of the perlcritic policies I use report problems because its PPI parser doesn’t recognize the new syntax. I could add exceptions in the perlcriticrc file and litter my code with more ## no critic annotations than it already had, but at this point, it was easier to just disable it entirely.

Another thing I had to disable for now was my Dist::Zilla::Plugin::Test::UnusedVars-generated Test::Vars test for detecting unused variables, as it reports multiple failures for the hidden @(Object::Pad/slots) variable. It does have options for ignoring certain variables, though, so I can explore using those and possibly file a pull request to ignore that variable by default.

Conclusion: The future looks bright

Overall I’m satisfied with Object::Pad and by extension some of the syntax that Corinna will introduce. I’m going to try porting the rest of dbcritic and see if I can work around the issues I listed above without giving up the kwalitee improvement tools I’m used to. I’ll post my findings if I feel it merits another blog.

What do you think? Is this the future of object-​oriented Perl? Let me know in the comments below.

Posted on
by
iceberg in body of water

We have a huge codebase of over 700,000 lines of Perl spread across a couple dozen Git repositories at work. Sometimes refactoring is easy if the classes and methods involved are confined to one of those repos, but last week we wanted to rename a method that was potentially used across many of them without having to QA and launch so many changes. After getting some help from Dan Book and Ryan Voots on the #perl libera.chat IRC channel, I arrived at the following solution.

First, if all you want to do is alias the new method call to the old while making the least amount of changes, you can just do this:

*new_method = \&old_method;

This takes advantage of Perl’s typeglobs by assigning to the new method’s name in the symbol table a reference (indicated by the \ character) to the old method. Methods are just subroutines in Perl, and although you don’t need the & character when calling one, you do need it if you’re passing a subroutine as an argument or creating a reference, as we’re doing above.

I wanted to do a bit more, though. First, I wanted to log the calls to the old method name so that I could track just how widely it’s used and have a head start on renaming it elsewhere in our codebase. Also, I didn’t want to fill our logs with those calls—we have enough noise in there already. And lastly, I wanted future calls to go directly to the new method name without adding another stack frame when using caller or Carp.

With all that in mind, here’s the result:

sub old_method {
    warn 'old_method is deprecated';
    no warnings 'redefine';
    *old_method = \&new_method;
    goto &new_method;
}

sub new_method {
    # code from old_method goes here
}

Old (and not-​so-​old) hands at programming are probably leaping out of their seats right now yelling, ​“YOU’RE USING GOTO! GOTO IS CONSIDERED HARMFUL!” And they’re right, but this isn’t Dijkstra’s goto. From the Perl manual:

The goto &NAME form is quite different from the other forms of goto. In fact, it isn’t a goto in the normal sense at all, and doesn’t have the stigma associated with other gotos. Instead, it exits the current subroutine (losing any changes set by local) and immediately calls in its place the named subroutine using the current value of @_. […] After the goto, not even caller will be able to tell that this routine was called first.

perlfunc manual page

Computer scientists call this tail call elimination. The bottom line is that this achieves our third goal above: immediately jumping to the new method as if it were originally called.

The other tricky bit is in the line before, when we’re redefining old_method to point to new_method while we’re still inside old_method. (Yes, you can do this.) If you’re running under use warnings (and we are, and you should), you first need to disable that warning. Later calls to old_method will go straight to new_method without logging anything.

And that’s it. The next step after launching this change is to add a story to our backlog to monitor our logs for calls to the old method, and gradually refactor our other repositories. Then we can finally remove the old method wrapper.