Refactoring as Part of Porting PHP4 to PHP7

I’m writing this pretty late, after a night of coding.

The LA IMC software port went kind of okay, but also kind of screwed up. There are a bunch of bugs popping up, but that was expected.

The first bug was simple: some of the code was still using the old classes, and some of the code was using the new classes. I had to search through the code to find out why this was happening, mainly by looking at the code.

Then, once the application was using the new code, I could fix the bug… which was in the new code, of course.

The harder bug was a weird case of a disappearing dollar figure. If the text had $nn, it would be deleted.

I managed to trace it down to somewhere between pulling data from the database, and handing it off to the code that displays the text. Even that was hard, because the code had many spaghetti-like qualities.

It was time to refactor.

Unfortunately, like most refactoring tasks, this wasn’t going to be easy. The old class, Article, was a huge mish-mash of code, and it was also a “God class”. It contained almost 1,500 lines of code.

I started to break it into different sub-parts, and here are the stats so far.

This is output from wc. The first column is the number of lines in the file.

    1496    4584   64323 Article.php  # the original
    1032    3136   44477 ArticleController.php
     149     519    4876 ArticleGateway.php
     106     333    3872 ArticleGatewayAccelerator.php
      65     163    1950 ArticleModel.php
     431    1046   15581 ArticleViews.php
      86     242    4069 Mangle.php
      58     158    2017 Nofollow.php
      66     184    1744 Tools.php

So, we’re getting more lines of code, overall, but the code is a lot easier to read.

Normally, when I’m refactoring, I do an initial “break” to restructure the code so it’s testable, and then write tests. This code base could not be tested that way.

So, I decided to carve the code apart, without testing.

Separation of Concerns

My technique is to copy the Article.php file to the new filename, and then start deleting code from the new file. I avoid reordering the lines, creating too many new functions, or adding code.

Some classes are pretty basic: ArticleGateway was straightforward, and encapsulated database access; ArticleViews is in progress, but, again, simple, because it just wraps all the views.

Some classes are hard. What’s in the Controller? What’s in the Model? I decided the model should take care of changes to the Article’s state, and pass on changes to the gateway.

Everything else will go into the Controller… or split into more classes.

Leave a Reply