Painless re-factoring of a class hierarchy
I had the following inheritance tree:
Environment______SpatialEnv______GridEnv
This was wrong. SpatialEnv implemented a complex plane, which GridEnv just did not need: it is a grid, after all!
What I really wanted was:
___GridEnv
|
Environment______SpatialEnv___|
|
|___PlaneEnv
Where PlaneEnv would implement the complex plane, and SpatialEnv would just contain a few methods, and mostly act to indicate that space exists (in some form) in a model. (Some code-checking program could test isinstanceof(SpatialEnv) to see if we are in space.)
But there were already a bunch of models using both SpatialEnv and GridEnv. How to slip this new class in the heirarchy without creating a lot of bugs?
I first simply copied the file containing SpatialEnv to a new file, and renamed the class PlaneEnv. At this point, there are no code changes. I then went through every model that used SpatialEnv, and changed it to use PlaneEnv. Then I ran each one to confirm they all worked... which it was pretty certain they would, since they were running the same code as before. (You can always miss something subtle that could go wrong, which is why I wanted to test.)
Now I made PlaneEnv a descendant of Spatial Env, and tested again. Everything was still working, but of course all of my code for PlaneEnv and SpatialEnv is doubled up at this point. So next I brought up all three of these classes on screen, and went through SpatialEnv method by method: if GridEnv used a method, it remained in SpatialEnv, and was deleted from PlaneEnv (since it would inherit the method from SpatialEnv.) If GridEnv did not use it, it was deleted from SpatialEnv and left in PlaneEnv. After each method was placed in one or the other class, I re-ran all of my models again. That way, if anything broke, I knew exactly which change had broken it.
I re-arranged this whole hierarchy while producing only one bug... and that was due to a typo. The key was proceeding systematically, instead of just hacking, as I might have in the olden days. Especially crucial was testing after every change was completed.
Environment______SpatialEnv______GridEnv
This was wrong. SpatialEnv implemented a complex plane, which GridEnv just did not need: it is a grid, after all!
What I really wanted was:
___GridEnv
|
Environment______SpatialEnv___|
|
|___PlaneEnv
Where PlaneEnv would implement the complex plane, and SpatialEnv would just contain a few methods, and mostly act to indicate that space exists (in some form) in a model. (Some code-checking program could test isinstanceof(SpatialEnv) to see if we are in space.)
But there were already a bunch of models using both SpatialEnv and GridEnv. How to slip this new class in the heirarchy without creating a lot of bugs?
I first simply copied the file containing SpatialEnv to a new file, and renamed the class PlaneEnv. At this point, there are no code changes. I then went through every model that used SpatialEnv, and changed it to use PlaneEnv. Then I ran each one to confirm they all worked... which it was pretty certain they would, since they were running the same code as before. (You can always miss something subtle that could go wrong, which is why I wanted to test.)
Now I made PlaneEnv a descendant of Spatial Env, and tested again. Everything was still working, but of course all of my code for PlaneEnv and SpatialEnv is doubled up at this point. So next I brought up all three of these classes on screen, and went through SpatialEnv method by method: if GridEnv used a method, it remained in SpatialEnv, and was deleted from PlaneEnv (since it would inherit the method from SpatialEnv.) If GridEnv did not use it, it was deleted from SpatialEnv and left in PlaneEnv. After each method was placed in one or the other class, I re-ran all of my models again. That way, if anything broke, I knew exactly which change had broken it.
I re-arranged this whole hierarchy while producing only one bug... and that was due to a typo. The key was proceeding systematically, instead of just hacking, as I might have in the olden days. Especially crucial was testing after every change was completed.
You a beast, Gene. =D
ReplyDelete"The key was proceeding systematically, ... crucial was testing after every change was completed."
ReplyDeleteWords of wisdom.