Friday 19 December 2008

Refactoring Hell

If there is some kind of coding hell, then refactoring is in the centre of the ninth circle of it.

I'm in the process of redesigning the Angband projection function, I dunno, for the fourth time in order to avoid unnecessary code duplication when it comes to applying magical effects to regions. I'm going to end up with a wrapper called project() which calls project_shape() to define a list of grids to affect, and then project_effect() which takes a list of grids and an effect, and applies that effect to the grids. This will allow me to take another list of grids - say a region - and use project_effect() directly to affect them.*

The problem I have with refactoring, is that you inevitably take a lot of hard won lessons and throw them away as a part of the refactor, and expose a lot of assumptions about how the code works to fundamentally different processes. And it's boring in the sense that at the end of it, the player doesn't notice any changes in the game, except more bugs.

So I'm tempted just to duplicate the code instead and move on. Especially as, having completed the refactor, I can get Unangband to reliably crash on my Windows box, which I can't get the mingw32 debugging environment working on for some reason - which means I'll have to debug on the Macbook instead. And do a sneakernet sync of the code repository to avoid polluting SVN with the offending code.

Ugh.

* The reason I have to take this approach, and can't do a single 'apply magic effect to a single grid' routine instead is quite subtle. One of the things magical effects can do is move you around e.g. teleport, waves, wind, gravity etc. But these effects can also potentially do damage. And I have to be careful that I don't move the player into another affected grid, and consequently double-hit them with a single magical attack (because of how important hit points are in a permadeath game, it'd be unfair to the player). So I've adopted the Sangband solution, which is to apply all the movement effects last, after all the other spell effects have occurred.

The Angband projection function already has this layering approach, applying effects to all the terrain first, then monsters, then the player. Sangband adds teleportation effects. And Unangband adds item breaking effects at the end, as items can explode to recursively create further projections, and I take advantage of tail recursion to ensure this happens in a safe manner. This means there is quite a bit of code to duplicate, which is why refactoring is a tempting trap to fall into.

3 comments:

Cyranix said...

Yeah, I'm sorry that you don't enjoy refactoring -- it's one of my favorite parts. I'm the kind of guy who likes editing papers, too.

Naturally, my bosses at the small software company where I work are of the "don't fix it if it's only partially broken" school of thought... grrrr...

Darius said...

"The problem I have with refactoring, is that you inevitably take a lot of hard won lessons and throw them away as a part of the refactor..."
I always thought the opposite. Refactoring gives you the chance to take those hard learned lessons and apply them the way you wish you had the first time. Still a lot of work though, and the other factors you mentioned are very true.

Great blog by the way. I'm a regular reader, and decided I'd start submitting comments.

Andrew Doull said...

Jussi: Branches in SVN are a pain. In fact, anything in SVN is a pain, but at the moment, transitioning to WinGit is potentially more painful. I'd have to get a new host for the sources for a start. (Just to clarify by a pain - I mean I'd actually have to learn and understand SVN as opposed to using it in a braindead fashion).

It's worthwhile emphasising that I'm refactoring here not because there are bugs in the existing code base, but because I'm trying to hold myself to the ideal of unnecessary code duplication. Which is the worst reason.