Sunday, February 18, 2007

Put Code Where It Belongs

Kingdoms of Arcania feedback has been coming in, and one of the requests was to turn off the banners for controlled pop centers. The banners will still be displayed in the pop center tool tips, but will not be rendered on the map itself. I decided that the boys should nap long enough today for me to implement this - it's a simple if statement to prevent the rendering in KOA, right?

Well, the solution is that simple, but the process turned into a game of Hunt the Wumpus. All the pop center rendering code is contained in one class... or so I thought. A quick read through the code turned up an icon rendering method that includes: if a banner exists, render it. I commented out the line and ran the game to see the effects. None.

Scratching my head, I took a look around, and realized this line controlled the tool tip banner rendering, but not the flags that are displayed on the map. Several frustrating minutes later, I found the following piece of code in an unrelated class:

if( actor instanceof PopCenter ) {
... draw Banners ...
}

After a few minutes studying the code, I was able to apply an Extract Method and Move Method refactoring (with a few other tweaks to make the refactorings possible). This is the frustrating part. A few seconds thought when this code was written could have saved me a great deal of precious time today.

I've been guilty of this type of code in the past. This will serve as a reminder to myself:

"Whenever you're tempted to add instanceof to the code, you're missing an opportunity to improve your design."

0 Comments:

Post a Comment

<< Home