Thoughts of a Flying Sheep

Recap
 Last time, we ended with:
 "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. 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. "

Steps
 There are a bunch of steps in this process:
1. CanonicalStatistics object changes (make AddStatistic method private, load on start-up)
2. Character object changes (give it its own CanonicalStatistics object)
3. SetupTest changes (CreateCanonicalStatistics no longer necessary. Just New Class)
4. Test and clean up

 I'm worried that these are a lot changes to make before any tests will pass, and I'm worried about the vagueness of step 4. I also feel like making changes that are not easily reverted or rolled back. There's probably a safer way to do this, but I'm going to proceed recklessly. Wish me luck.

1. CanonicalStatistics object changes (make AddStatistic method private, load on start-up)
 Ok, changes made, and (from habit), I run my tests. First failure (clearly) is a call to the now private AddStatistic method. I have three choices:
 (a) make the function public again (to avoid runtime errors)
 (b) set the tests to treat runtime errors as failing tests.
 (c) use the errors to local and eliminate the errors.

 I could use a combination of (b) and (c), but I'll just do (c), as it gives me line numbers (: I will admit temptation for doing (a), but, to my credit, that temptation dissipated quickly.
 Actually, ConfirmCanonicalStatisticUnicity test is no longer required. I'll nuke it. Interesting - it was the only one to use the AddStatistic method. Cool.

3. SetupTest changes (CreateCanonicalStatistics no longer necessary. Just New Class)
 Did these as part of 1, to allow me to test. I will, however, remove the unnecessary function.

2. Character object changes (give it its own CanonicalStatistics object)
 Phew! As it turns out, my character object already had a function named FindStatisticByName, so trying to make it call a global function of the same name would have been a disaster.

4. Test and clean up
 My gosh! Other than a single copy/paste error, all my tests are passing now! A change that I approach with (justified) apprehension seems to be working.
 Consider what would happen in a world without tests: I'd make the changes, but be convinced that there was a bug lurking somewhere. Now I know that, even if there is a bug left in the code, it doesn't affect anything I care about (e.g. anything that I've written a test for). Instead of worrying, or re-reading the code, or even proceeding with unfounded confidence, I can simply resume coding - knowing that my tests all pass.

Mission Accomplished
 Indeed, I could even stop here and pick up again on another day (: I'm going to review my todo list and see what appeals to me. At least my code no longer smells of Global variables and test dependencies. Although, to close the loop, I should add a tear-down function.
 Done. This reminds me, though, that I also need to add Deconstructors to my classes to free up arrays that are created. I've added that to my todo list, but near the bottom.

Additional parameters
 Note that I've just fixed some of the tests that were named CreateFoo, but were really CreateAndTestFoo. I've done this by separating the two functions, adding Foo to the parameter list, and passing it to the testing function. I should do this for each of my basic test Characters. So far, I have:

Character without Statistics or Abilities
Character with Statistics, but no Abilities
Character with Statistics and Abilities, but low intelligence
Character with Statistics and Abilities, and high intelligence

 The first two, honestly, aren't interesting. They are really edge cases, and I don't think those tests add a lot of value, so I'm going to delete them.
 Actually, not having an ability but trying to use it might be a useful case. I'll leave that one for now.
 So, what I'd like to do is create two characters to use as parameters. One will be minimal (low statistics in everything), and the other will have maximal statistics. I need names for these characters - how about Homer (min) and Lisa (max)? Perfect.
 I'll add these characters to the setup function and then make the test functions use them (instead of creating their own characters).

Creating Homer
 Now that I have this CanonicalStatistics object, I should use it to loop through all statistics to create (and test) these characters. I'll have it expose its AllStatistics array (for read only).

Function Names
 One nice thing about this is that I can replace testCharacterWithLowIntCanNotReadBook with the more readable testHomerCanNotReadBook.

Simpsons
 Ok, both characters are in place, and are being used by the test functions. All tests pass, the code is much less stinky now, and I think this is a good place to stop for today.

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...