いい加減なシステム会社のいい加減なソースをリファクタリングする
出先はシステム会社がVB6で作ったシステムを使っているのだが、あまりにシステム会社が駄目なので私が途中からソースを引き取って完成させたという代物。
今でも私がメンテナンス（機能追加など）を請け負っている。
どのくらいダメかと言うと、
- 何度かの打ち合わせを経て先方から「開発期間は2007年12月〜2008年3月、リリースは2008年4月1日」というスケジュールを提示されたのに、リリース日時点で（今から考えれば）30%程度しか出来ていなかった
- 仕様の確認が甘く「そうじゃない」と指摘する事案が多発*1
- 2010年1月時点でも完成度が70%ほどだった
結局2010年2月から私が開発を引き継いで根幹部分をほぼ作り直したりしながら完成させたのだが、それでも正しく動いているところは基本的にそのまま使っていた。
で、今回チェック機能を追加しようとして当該箇所のソースを見てうーむ。
Public MSG_RET As Integer Private Sub cmdNo_Click() Me.Enabled = False DoEvents MSG_RET = 2 Unload Me End Sub Private Sub cmdOK_Click() Me.Enabled = False DoEvents MSG_RET = 0 Unload Me End Sub Private Sub cmdYes_Click() Me.Enabled = False DoEvents MSG_RET = 1 Unload Me End Sub
ええと、これ、素人さんが作ったのかな？
ソースの先頭に作成者の名前が書いてあるけど、この人ベテランだよね？
前のシステムに携わっていた人だから、今はともかく以前はバリバリのプログラマだった人だよね？
出先が相当甘く見られていたのか、「動けばメンテはどうでもいい」の精神なのか。
行数で単価が決まっていた名残なのか。
単純に置き換えるなら、こんな感じ？
Public MSG_RET As Integer Private Sub cmdNo_Click() Call cmdClick(2) End Sub Private Sub cmdOK_Click() Call cmdClick(0) End Sub Private Sub cmdYes_Click() Call cmdClick(1) End Sub Private Sub cmdClick(rtn As Integer) Me.Enabled = False DoEvents MSG_RET = rtn Unload Me End Sub
で、0とか1とかだと分かりにくいから、
Public MSG_RET As Integer Private Const MSG_RET_OK As Integer = 0 Private Const MSG_RET_YES As Integer = 1 Private Const MSG_RET_NO As Integer = 2 Private Sub cmdNo_Click() Call cmdClick(MSG_RET_NO) End Sub Private Sub cmdOK_Click() Call cmdClick(MSG_RET_OK) End Sub Private Sub cmdYes_Click() Call cmdClick(MSG_RET_YES) End Sub Private Sub cmdClick(rtn As Integer) Me.Enabled = False DoEvents MSG_RET = rtn Unload Me End Sub
みたいにしてあげると親切かなぁ。（時と場合による）
私は正直プログラミング能力がそれほど高い訳ではないけど、これくらいは「わざわざそうする」というより「最初からそうする」レベルのように思うのだけれど……。
そちら様はシステム会社なんですよね？
うち（出先）からお金をもらってシステム開発したんですよね？
と言いたくなるなー、と久しぶりに思ったのであった。
