■

しごと

ExcelのVBAの仕事の中で、
「現在動いているものですからサンプルに〜」
っといただいたものがあった。
コードを開いたら・・・・・・

Sub Edit_Proc2(Flg As Integer)

　　Dim i As Long
　　Dim j As Long
　　Dim k As Long
　　Dim m As Long
　　Dim n As Long


Σ(ﾟдﾟ　)！？！？


ソースを開いてわずか1秒。
目の前がクラクラしました。


この後のソースを見るまでも無く断言できる。

(,,ﾟДﾟ)＜これはクサレコードだ


たった7行ですが以下の状況により次のことが推測できます。

  1. Function名が連番名になっている
  2. おそらくループカウンタが5つもある
  3. その全てが1文字変数である

これらより

  • 2より、このFunctionの行数はだいぶ長い
  • 同じようなFunctionが複数ある
  • サブルーチン化などされてようはずはない
  • 共通化なんぞ夢のまた夢
  • 1より、連番変数も使われていそう
  • 1と2より、その数も多いだろう
  • 2より、2重3重当たり前の多重ループがありそう
  • 3より、ほかの変数名もロクなことにはなってなさそう
  • 行そもそもコメントがないことから、行き当たりばったりなソースが記述されていそう


心温まる推測状況がかけらも見当たりません。


ちょろっと答えあわせをするべく中身を覗いてみてみたら。


このFunctionは820行ほどあり。

　　Dim SheetName1 As Object
　　Dim SheetName2 As Object
　　Dim SheetName3 As Object

というこれまた殴り倒したくなるようなネーミングがあり。
処理の区分けにラベルが使われておりGoto制御。
Cell位置に名前をつける努力の跡がみえる変数はよいけれど、
それ以外のマジックナンバーの多いこと多いこと。
ちょいと伏せるとこ伏せるけどこの3重ループ。

　　For i = 2 To 60000
　　 If 判定 Then
　　　　 For j = i + 1 To 60000
　　　　　　 If 判定 Then
　　　　　　　　For k = i To j
　　　　　　　　　　'処理〜
　　　　　　　　Next k
　　　　　　　　i = j
　　　　　　　　Exit For
　　　　　　 End If
　　　　　　
　　　　　　 If SheetName1.Cells(j, 1) = "" Then
　　　　　　　　Exit For
　　　　　　 End If
　　　　 Next j
　　 End If
　　
　　 If SheetName1.Cells(i, 1) = "" Then
　　　　 Exit For
　　 End If
　　Next i

(,,ﾟДﾟ)＜内側のループの中で外側のループカウンタ書き換えんな
(,,ﾟДﾟ)＜ってか60000ってなにその中途半端に多い限界値


こんなFunctionが3つもありました。


こりゃこえーと思ってほかもよくよくみてみたら、でるわでるわ。
特にひどいのこれ

　　For i = i To 65536
　　　　'70行くらいのソース

　　　　 For n = i To 65536
　　　　　　 If 判定 Then
　　　　　　　　Exit For
　　　　　　 End If
　　　　 Next n
　　　　
　　　　 i = n

　　　　 If n = 65537 Then
　　　　　　Exit Sub
　　　　 End If

　　　　 j = j + 1
　　　　 m = m + 1
　　　　 If SheetName3.Cells(m, 7) = "" Then
　　　　　　Exit For
　　　　 End If
　　　　'350行くらいのソース
LBL1:
　　　　'130行くらいのソース
　　Next i

(,,ﾟДﾟ)＜「n = 65537」になる条件ってなによ！
(,,ﾟДﾟ)＜なんで560行を超える巨大なループやねん
(,,ﾟДﾟ)＜途中にラベルがあるってどうよ
なによりこれ。
＜怒り＞
(#ﾟДﾟ)＜1文字変数の使いまわしﾔﾒﾛ！！
＜／怒り＞

説明受けている時に聞いた言葉がよみがえってきました。
( ´∀｀)＜これね〜。○○会社に入れたときにそこ専用にガッツリ組んだらしいんですよ〜。
　　　　　なんで汎用性なしなんで、今回汎用的にしたいんですよね〜。


今なら実感を伴って改めていえる
(,,ﾟДﾟ)＜激しく同意だｺﾞﾙｧ