Enhancing by refactoring

Two years ago I read a lot about OOP. Every time I picked a new concept in some forum or chat I’d do my best to understand and apply it. I was eager to improve my coding knowledge and skills but everything we code as a purpose and an audience and if we forget the purpose or the audience we are introducing more problems with the knowledge we have than solving problems.

That happened with Bold Pixel. At some point in time Bold Pixel became a monster to please developers instead of a tool to be more productive in my purpose to my audience: create games for players! People play games, not code!

I stopped over-engineering sometime ago and started reading about how to become a better programmer not by writing better code, but by writing the exact code to solve the exact problem. This brought a new problem: what to do with Bold Pixel and how to maintain and evolve it? First thing was to start a new package and only bring in classes that I needed but wire the classes up to be as slim and straight to the point as possible. I do that all the time to build up my code base right now, but that’s not all. Here’s the whole process I’m using right now.

Start simple

 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
class BlitObject {
    // Position
    public var x:Number = 0;
    public var y:Number = 0;
    // Rendering
    public var visible:Boolean = true;
    public function BlitObject():void {
        // Constructor
    }
    internal function render():void {
        // Render stuff here
    }
}

The class above can be a stub for a simple blit object. From a OOP perspective it breaks encapsulation completely since x, y and visible variables should be private but seriously, do I really need those to be private or protected and have getters and setters? No I don’t so no need to over-engineer it. The only thing I want to have contained is the render method that should only be called by the blit engine that lives in the same package.

The blit object only responsibility is to copyPixels or draw on a given world position translating camera position on the fly.

Writing a simple class like this is fast, performs fast, case closed.

Evaluate new needs

Now I need a tile map class. This class is also a blit object so following the “is a” versus “has a” rule tile map should inherit blit object. Considering the blit engine has a camera, the tile map won’t move. This brings two new problem.

1. If the tile map doesn’t move, why do I have x and y publicly accessible?
2. I want my camera to follow blit objects so blit objects must have x and y variables (or properties). How can I make tile map be recognized as a blit object, have x and y and have none of those altered?

Sounds messy…

I could write a Blittable interface that would define a render function and getter and setter methods for x and y. The problem is that if there’s something I want to keep in the package scope is the render function and interfaces define public interfaces.

The solution is to convert blit object in an abstract class and have its original functionality moved to a new graphic class.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
class BlitObject {
    // Rendering
    public var visible:Boolean = true;
    internal function render():void {
        throw new Error("Abstract function called. Override render()!");
    }
    public function get x():Number {
        return NaN;
    }
    public function set x(v:Number):void {
        throw new Error("Abstract function called or property x not available.");
    }
    public function get y():Number {
        return NaN;
    }
    public function set y(v:Number):void {
        throw new Error("Abstract function called or property y not available.");
    }    
}
class Graphic extends BlitObject {
    // Position
    private var _x:Number = 0;
    private var _y:Number = 0;
    override internal function render():void {
        // Render stuff here
    }
    // Position
    override public function get x():Number { return _x }
    override public function set x(v:Number):void { _x = v }
    override public function get y():Number { return _y }
    override public function set y(v:Number):void { _y = v }
}
class TileMap extends BlitObject {
    override internal function render():void {
        // Render the tile map
    }
}

Problem solved. The class responsibilities are much better defined.

1. A blit object renders and has x and y coordinates. Since it is an abstract class everything we try to do with it directly will result in an error.
2. A graphic is a world movable blit object that renders by copyPixel or draw.
3. A tile map is a world static blit object that renders very large objects by copyPixel or draw  based on a predefined xml specification.

Conclusion

What’s the conclusion here? Well for starters by this time I’d have a couple of interfaces and probably a helpers. I’m not saying it is wrong to have interfaces and helpers, actually it is preferable, but it would be coding for the sake of coding instead of coding for the sake of making a game. It took me 10 minutes to do this and while I was at it the BlitObject class also extends a ListObject class that really speed up the iteration, addition and removal of objects from the blit engine.

Still… I did repeat my self and the code has suffered another refactoring since then. I’ll leave that for another post but I wonder if you know what’s repeated… :)

Posted: January 28th, 2012
at 2:00am by Vlad

Tagged with , , , , ,


Categories: Dev Journal: Danger Zone,The code of VGS

Comments: No comments