Sunday, March 18, 2007

Cleaning and Course Correction

Clean Working Environment
 At some point, I'm going to need to clean off my (physical) desktop. There's barely room for my cereal and tea. The real "clean environment" that I have to work with is my set of tests, all of which pass. It's a lot easier to motivate myself to extend something that's working (rather than fix something that's not working).

Hats
 The last session results in a lot of additions to my todo list, as well as a number of things I'd like to do differently. Martin Fowler's book (Refactoring) discusses a programmer "wearing different hats" - one for extending functionality, and another for refactoring. It simplifies things to think of them as separate: you refactor so that you can add functionality easily, but you're not changing functionality as you refactor. I hope that's clear.
 So yesterday was Add functionality (and refactor when necessary), and, as a result, the code has that awful "Global Variable" smell, as well as "Tests have undocumented dependencies" smell. Both of which I'm going to try to fix today. In other words, today will be Refactor away Smells (and possibly add some functionality when finished).
 But first - catch up on emails and blogs. No music this morning, as my wife and visiting father-in-law are still asleep.

Setup/Teardown
 The first thing is to create setup and tear down functions, which remove the dependency on the CreateCanonicalStatistics function.
 That was easier than expected. But I'm not done yet. The problem is that CreateCanonicalStatistics is really CreateAndTestCanonicalStatistics, which is too much for it to do. I'm going to break some of these functions (also CreateBasicCharacter) down, and pass the parameters as an array instead. So, setup functions will create the array, and then all functions will get the array (even if they choose not to use it). Actually, strike that - only the functions that choose to get the array will get it. I don't think it's too difficult for a function to opt-in.
 Of course, in a real programming language, I'd just make the array optional.
 Additionally, I will pass a copy of the array (not the array itself). If one of these functions tries to modify the array, it will still be in its original shape for the next test.
 So, I'm going to copy the existing CreateCanonicalStatistics function, rip out the testing bits, and add it to the setup routine. Also renamed the existing function to TestCanonicalStatistics, and updated it to add a parameter.
 The SetupTests function will be the only function allowed to modify the array of parameters.
 Another unexpected failure! Great! (: It's nice to know that these tests are not a waste of time (: Error was easily fixed.

Extraction success
 Ah ha! I did it. I removed the global variable from the Create function, moved it into a setup function, and passed it to the test function, which worked like a charm.
 Of course, the tests that were using the global variable are now failing, but they will be easy to clean up (same process as cleaning the TestCanonicalStatistics function).

Snag!
 Uh oh. Character.UseAbility also relies on this global variable. At this point, I'm beginning to think that I might be spending too much time on an object whose sole purpose is to be a stand in for an actual database table. It's time to think about alternatives.
 The one that appeals the most to me at the moment is to stuff the statistics (and their IDs) in the Application object (which has global scope). I can then expose global functions for adding/finding the statistics by ID or by name. Ultimately, I won't need the CanonicalStatistics class at all.
 Well, I've got my failing test, so I can start by replacing objCanonicalStatistics.FindStatisticByIndex with a global function FindStatisticByIndex that returns "Strength". This will at least make the tests fail without throwing runtime errors.

Runtime Errors
 Somehow, runtime errors strike me as worse than failing tests. Is that a reasonable preference on my part?
 Anyways, I've sorted all the runtime errors, but I haven't finished implementing the real versions of the functions to store and retrieve values from the application object. I think that the syntax should be something like:

Application("Statistic_1") = "Strength".

 In this case, retrieving the name from the ID is easy, but retrieving the ID from the name is difficult. Perhaps I should also include:

Application("Statistic_Strength") = 1.

 Of course, I hate having data in two places, as I fear it may get out of sync. Perhaps I should simply store the array that used to be in the CanonicalStatistics object, and use the same logic as I was using before. Or, possibly, I should just give my Character object a local copy of the CanonicalStatistics object.

Changing Course
 Ok, I think I'll take that route - Any object that needs to know about statistics will include a local objCanonicalStatistics, which will be loaded when the object is created. It can then use that object to resolve references by name or by ID. At least, we'll give that approach a try for now, and see where it goes.
 As part of this, the objCanonicalStatistics will no longer expose a public AddStatistic method. Instead, it will load all the statistics it knows about when it's constructed.

To be continued...

0 Comments:

Post a Comment

<< Home