Most important bits of refactoring complete!

Finally, after what seems like an eternity, I have completed renaming a load of functions to use a nicer more Perl-ish form. It’s taken 3 attempts, but I’ve finally got there and hopefully I haven’t broken anything. I’ll just take a moment to explain how I’ve done the refactoring, so hopefully some veteran coders can tell me how I should have done it 😉

I worked through modules to decide which functions to rename – let’s start in MusicBrainz::Server::Artist for example. I want to rename “sub GetResolution” and “sub SetResolution” into just “sub resolution.” Firstly, I did:

grep -r ‘sub [GS]etResolution’ .

This showed only the getter and setter I was interested in. If it showed more complicated functions (i.e. sub GetResolutionInReverse) then I would rename those first. Next, I would do the bulk of the renaming:

find lib -name ‘*.pm’ | xargs perl -pi -e ‘s/[GS]etResolution/resolution/g’

And finally, I went back into Artist.pm and rewrote sub resolution to look like:

sub resolution
{
    my ($self, $new_resolution) = @_;

    if (defined $new_resolution) { $self->{resolution} = $new_resolution; }
    return $self->{resolution};
}

I did this over just about every function that my facades are using, so I can now merge this refactoring and make a new branch to delete those facades (never liked ’em in the first place!). And just to give an idea how much code this touched:

musicbrainz@doughnut ~/TemplateToolkit/lib $ git diff --shortstat master..HEAD
120 files changed, 2549 insertions(+), 2584 deletions(-)

And one final statistic…

musicbrainz@doughnut ~/TemplateToolkit/lib $ grep -r 'sub [GS]et' . | wc -l
255

255 functions still have Get or Set in front of them – at some point these need to be changed! But this will likely happen after facades are removed – one step at a time.

A slow week

Those who follow the mb-commits mailing lists, will probably have noticed a slow down in my commits, so I wanted to explain what was up 🙂

We made a decision to remove the current system of using Facades with Models, to just rewriting the entities to have the correct interface. This mean renaming all getters and setters to just accessors (much akin to the interface Class::Accessor provides) and is an incredibly slow and draining process.

I have a local branch from my Git copy of the Subversion branch that I’ve been commiting too – but as said, progress is slow. It is getting there of course, and I still think it will be worth it when it’s done, mainly due to cleaner and more consistant interfaces, and also reducing the size of the code.

That is all 🙂