Craft · September 27, 2016 · 9 min

See email from Casey

PHP Git

---

The email is gone. Nobody at the client remembers a Casey, the developer who left the comment moved on sometime in 2015, and the comment is dated April 2014. So I’m not sure who I’d even ask.

Here’s the comment. It’s buried in the middle of the longest function in the project, sitting on top of the pricing rule nobody has been willing to go near since.

wp-content/themes/store/functions.php

// do not change, see email from Casey 4/14
// wholesale is 18% off, and then there are the exceptions
if ( in_array( 'wholesale', (array) $user->roles, true ) ) {
    $price = $base * 0.82;

    if ( has_term( 'clearance', 'product_cat', $product_id ) ) {
        $price = $base;
    }

    if ( in_array( $brand, array( 'ridgeline', 'kestrel' ) ) ) {
        $price = $base * 0.95;
    }
}

PHP

I took the store over in March. Somebody else had been maintaining it since 2012, so what I actually inherited is a platform plus four years of one person’s opinions about the platform. The theme’s functions.php is around 1400 lines of PHP, and the function in the middle of it, the one every page ends up calling, is 211 lines long. I know that because I counted them, which is what you do in the first hour when what you’re really doing is having a feeling.

Four levels of nesting and a $tmp2

The feeling was contempt, obviously. It’s 211 lines long and there are no tests anywhere in the project. It nests four levels deep in places, and there’s a variable in it called $tmp2 sitting a little way below a variable called $tmp.

And I’ve done this enough times now to know the feeling is useless and also, most of the time, wrong on the facts. The people who wrote this were not worse at their jobs than me. They worked on the same code for years, under deadlines I know nothing about, against requirements nobody wrote down anywhere I can find, and every strange thing in that file is a fence standing in a field where I can’t see the animal. That’s Chesterton again, who turns up in my head more often than a dead Edwardian essayist really should.

More to the point, somebody is paying us to change this thing, and contempt makes me want to rewrite it. A rewrite is the most expensive way there is to find out what a piece of software was doing.

So I’ve ended up with a fixed order I read things in, mostly to stop myself from having an opinion before I’ve earned one. It’s just always the same, which is the part that does the work, because the thing it protects me from is my own first hour.

The scenic route

Not the code. The site.

I click around it as a customer before the editor is open at all. What does it sell, what are the pages, what happens if I put a bad email in the checkout. You can’t understand the shape of a codebase without the shape of the thing it does. I have watched myself read three files deep into something that turned out to be a feature nobody has used in years.

Then the log, still not the code.

first hour

# what changes around here, and how often
git log --since="1 year ago" --stat | less

# how many people have been in here
git shortlog -sn

# is this file alive or fossilized
git log -1 --format=%cd -- wp-content/themes/store/functions.php

# who wrote the pricing rule, and when, and what they said about it
git blame -L 812,851 wp-content/themes/store/functions.php

Shell

This is the highest-yield hour available anywhere in a project and I ignored it for my whole first year. The log tells you things the code physically cannot. Which files get changed together, and are therefore secretly one thing. Which file gets touched every other week, and is therefore where the work of this project actually happens. Which file hasn’t moved since 2013, and is therefore either solid or dead, and one grep tells you which.

Here it answered three questions inside the first hour. There are 11 contributors in the history and not one of them is reachable, which is a completely different project from two people who are both still on email. It changes what I’m allowed to assume about consistency. functions.php sits at the top of the change list by a distance, so the scary file is also the working file, which isn’t always true and is worth knowing before you plan anything. And the pricing rule hasn’t been touched since the commit that first put it there, in April 2014, while the data-gathering above it has been edited a dozen times since. One file, two speeds.

git blame -L takes a line range, which is how you ask about 40 lines instead of a file. The commit message on those 40 is updates, which is about what I deserved for hoping.

Then the manifest and the build. The package.json, the gulpfile, whatever config is lying around the root. Not to run any of it, just to read what the project thinks it is. A dependency list is a statement of intent, and where the intent and the code disagree, that gap is usually where the interesting history is.

Then the entry points. How does a URL turn into a file. The rewrite rules, the front controller, the template hierarchy, whatever the answer turns out to be here. It’s the only map that’s reliably accurate, and until I have it I’m reading paragraphs out of a book with no page numbers.

Then the templates, deliberately before the logic, because the output is the part I can check. I can load the page and find out whether I understood it, and being able to check yourself cheaply is worth an enormous amount when everything is unfamiliar.

And then, finally, the 211 lines. With a question.

Understanding is not a task

This is the part that took me longest and it’s the one I’d actually pass on.

“Understanding the codebase” has no edge and no finish line, and if that’s the goal you will read for three days, feel worse than when you started, and retain none of it. I’ve done exactly that and put the hours on a timesheet.

Turning up with something specific works. Why does the price on this one page come out without tax. Where does that confirmation email get sent from. What happens between somebody hitting checkout and a row appearing in the orders table. Then you read only what’s on that path, follow it end to end, and come out knowing one whole thing instead of nine partial ones.

Do that a few times on different questions and you’ve got the codebase sideways, in a way that actually stuck. It’s slower to feel like progress and quite a bit faster to be it.

Hands where I can see them

Refactoring while reading. This was my worst habit and it took a bad week to break it.

The urge is enormous, because tidying feels like progress and reading doesn’t feel like anything at all. So you rename $tmp2 while you’re in there, and pull a couple lines out into a helper, and by the end of the week there’s a diff across nine files that does nothing a client can see, sitting on top of no tests. You can no longer tell which of your own changes broke the thing that’s now broken.

Instead I keep a list in a markdown file. Every time I want to change something I write down the line number and what I’d do about it, and then I change nothing for the first two weeks. A good chunk of that list turns out to be wrong once I know why the code is like that, which is a very cheap way to be wrong.

When I did finally split those 211 lines up, it was because I had to change one part of them, and I only pulled out the part I was changing. The new behavior went into a new function that the big one calls, so the risky piece is small and isolated and I can look at it on its own. The rest of it is exactly as it was, being ugly and correct.

It does look wrong

For the record, that function is three things stapled together, which is what it always is. About 60 lines of collecting a product’s data, about 90 of formatting it for display, and 40 of a very specific rule about what one group of customers pays.

Casey’s email is gone and I’ve stopped looking for it. That comment is still the most valuable thing in the file, because it’s the only part of it that tells me a person was surprised here once. It’s sitting on the exact 40 lines I’d have cheerfully simplified in week one on the grounds that they look wrong. Those three words get read every time somebody opens that function, which is more than the style guide I built earlier this year ever managed.

They do look wrong. They’re also the business, and my sense now is that most of what makes an inherited codebase ugly is four years of decisions showing through, instead of anybody having been bad at this. None of the above is a methodology. It’s an order, and the order exists to buy me two weeks of not having an opinion, because the opinion I have in hour one is always the same one and it has yet to be right.