I think perhaps the logic is somewhat backwards here and causing the problem. What if your methods looked like this:
updateTo1002()
{
if (version != 1001) {
updateTo1001();
}
// do the job
}
updateTo1003()
{
if (version != 1002) {
updateTo1002();
}
// do the job
}
I don't know your exact use case, but it would seem to me like most often you would want to update to the most recent version, but install the incremental updates as needed along the way. I think doing it this way captures that logic better.
Edit: from @user470379's comment
In this case, mostly it's identifying the fact that you have a copy/paste pattern and editing it.
The coupling problem is, in this case, barely an issue but could be. I'll give you a few things that could come up in your scenario that would be hard to code if done this way:
- Every update now needs an extra cleanup step, so after updateTo1001() call cleanup(), etc.
- You need to be able to step back in order to test older versions
- You need to insert an update between 1001 and 1002
Let's take a combination of these two done following your pattern. First let's add an "undoUpgradeXXXX()" to undo each upgrade and be able to step backwards. Now you need a second, parallel set of if statements to do the undos.
Now, let's add to that "insert 1002.5". All of a sudden you are re-writing two potentially long chains of if statements.
The key indication that you are going to have these kind of problems is that you are coding in a pattern. Look out for patterns like this--in fact, one of my first indications is usually when I'm looking over someone's shoulder at their code, if I can see a pattern without even being able to read anything written like this:
********
***
*****
********
***
*****
...
then I know I'm going to have problems with their code.
The easiest solution is generally to remove the differences from each "group" and put them into data (often an array, no necessarily an external file), collapse the groups into a loop and iterate over that array.
In your case, the easy solution is to make each of your upgrade objects with a single upgrade method. Create an array of these objects and when it's time to upgrade, iterate over them. You may also need some way to sequence them--You're currently using a number, that might work--or a date might be better--that way you can "Go to" a given date easily.
A few differences now:
- Adding a new behavior to each iteration (cleanup()) would be a single line modification to your loop.
- Reordering would be localized to modifying your objects--possibly even simpler.
- Breaking your upgrade into multiple steps that must be called in order would be easy.
Let me give you an example of that last one. Suppose after all your upgrades have been run you need to go through an initialize step for each (different in each case). If you add an initialize method to each object then the modification to your initial loop is trivial (simply add a second iteration through the loop). In your original design you'd have to copy, paste & edit the entire if chain.
Combine JUST undo & initialize and you have 4 if chains. It's just better to identify problems before you start.
I can also say that eliminating code like this can be difficult (downright tough depending on your language). In Ruby it's actually pretty easy, in java it can take some practice and many can't seem to do it so they call Java inflexible and difficult.
Spending an hour here and there mulling over how to reduce code like this has done more for my programming abilities than any books I've read or training I've had.
Also it's a challenge, gives you something to do instead of editing huge if-chains looking for the copy/paste error where you forgot to change 8898 to 8899. Honestly it makes programming fun (which is why I spent so much time on this answer)
