[VBA]中規模のマクロツール開発後の大反省会

はじめに

人事部の方からマクロの作成を依頼されたため、3か月間かけてリリースまで行った。

着手するタイミングでは、個人的な感覚値で半分の1か月 ~ 1か月半で実装できると思っていた。

にもかかわらず、なぜこんなにもかかってしまったのだろう。思い当たる節は山のようにある。

次回につなげるために振り返っていく。

 

ツールの概要

エクセル(VBA)とPower Automate for Desktoop (PAD)の2点を使用。

エクセルからPADへブラウザ検索依頼を投げて、PADからエクセルへ結果を記載する。

その後、エクセルでなんやかんやの加工処理を行うというもの。

 

全体の作業量

全体の作業期間は正味の3か月ではあるが、テストを除いたコーディングに重きを置きたいので、テスト期間を省き賞味2か月で考える。

日数

作業した日数

  • 3月 ~ 4月
  • 出勤日は45日程度
  • まったくの別業務を行っていたのが12日ほど
  • 45 – 12 = 33日

1日当たりの実装時間

  • 繁忙期と閑散期の平均値をとって、4時間とする

作業時間の合計値

  • 33 * 4 = 132時間
  • 132 / 8 = 16.5日 (正味の営業日)

 

コーディング量

おおよそ、2500行程度、コメントと空行が15%を占めるとするならば

  • 物理的SLOCは「2500」
  • 論理的SLOCは「2000」
  • 1日当たりのSLOCは「120」

 

要因→改善の洗い出し

環境問題

肩こり

問題

肩こりというよりは、背中が固くなると、タイピングができなくなる。

ひどい時では、「お疲れ様です」このたった一言をタイピングするのですら億劫に感じるほどだ。

改善案

最近ストレッチをすることで、いくらか改善することが分かってきた。

土下座のポーズから、おなかが見えるように首をさらに丸めて地面に押し付けると、首から背中までが伸びてくれる。

 

職場の騒音

問題

職場には多くの職員がいるため、それ相応の騒音が発生するのはしょうがない。

ただ、自分の特性上、聴力のフィルター機能が弱いため、くしゃみ・咳払い・打鍵音・貧乏ゆすり・痰切り・談笑など、基本的には全てそのままの音として入ってくる。

一定時間そういった音を聞き続けると脳がフリーズを起こすことがある。

フリーズにも度合いはあるが、ひどいときはエクセルのシートから値を取得するだけの処理すらかけない。

適当にSheetとRangeを取得して、何らかの変数に代入すればいいだけなのにも関わらず。

改善案

今年中には病院に行きたい

 

レイアウト

1シートに機能を詰め込みすぎる

問題

・ユーザーがエクセルファイルを開いたときに、シートは最小限のほうが、わかりやすくてよいのではないか?と思い、ユーザーが操作する項目(ユーザーに向けた説明文やユーザーが押下するボタン)と作業に必要な表形式のデータを同じシートにしていた。

・また、それらの情報をなるべく1画面中に収めるために、セルの縦幅や横幅を可能な限り縮めたり、不要な行列を削り、情報を詰めすぎる事態に発展した。

・PADの処理結果をエクセルに逐次出力されることで、ユーザー側で進捗状況が把握できてよいのかなと思ったのも一つの理由だ。

 

改善

シートはなるべく1つのシートに1つの目的が収まるようにしたほうがいい。例えば、1つのシートに2つ以上のマスタ情報を記載することは原則あり得ない。

同じように、ユーザーが操作をするボタン等については、それ専用の操作用シートを用意するべきだ。そうすることで、操作用シートに編集が加わることで、列が変動し、ほかの全く無関係な項目の参照列へ影響し、開始列番号 = N の値をいちいち変える必要もなくなる。

 

PADからエクセルへ転記をするためのシートは必ずしも必要ではない

問題

「1シートに機能を詰め込みすぎる」と反するかもしれないが、これについてはケースバイケースだ。

一度は転記用のシートを用意したのだが、

  1. エクセルからPADを起動する。
  2. PADが処理結果をエクセルに出力する
  3.  1. ,2. をリストの件数分繰り返す。

1件の処理であれば問題なかったのだが、繰り返しの処理により相性が悪くなった。1.で次の1件を出力する前に、2.で出力した処理結果を退避しなければ上書きされてしまう。そのためには、エクセル側で退避させるための転記用のメソッドを用意し、それをPADから呼ぶ、またはエクセル側で無限ループし、セルの値を監視し、転記されたら自身で転記用のメソッドを呼び出す必要がある。

PAD側で全件処理をしてしまい、処理結果を一度に出力すればよいのではないか。については、出力結果の形式による。決まったフォーマットであればよいのだが、可変的なものであれば、(出力結果が件数変動のリスト形式など)それを全件数分出力するのは、フォーマットが決められないので難しい。

改善

PAD→エクセルの転記処理がどうしてもごつくなってしまう場合は、PAD側で加工処理をしてしまえばよい。PADは複雑なデータ加工には不向きだが、多少であればなんとかできる。そのため、あえて転記用のシートは設けずに、インプットとなるリストから全件インプット情報を取得し、テーブル形式で全件アウトプットするのが最も手っ取り早い。

 

レイアウトの体裁を整えすぎる

問題

初期の段階から、シートの色であったり、罫線であったりのレイアウトの体裁を整える(表の色を変えてみたり、罫線を整えたり)ことに注力しすぎた。

データ定義の変更が発生するたびにレイアウトの体裁を再度整えるという行為に対してかなりの時間を要した。テストの実施まで終えて、ようやく完成しました。のタイミングで1回だけレイアウトを修正すればそれだけでよかったにもかかわらず。

改善

実装中は、処理に影響しない見た目にかかわる部分はおざなりにしなければいけない。

さらに言うと、VBAがセルの値を取得するための参照座標も基本的には変えてはいけない。

「そのシートから取得する」ことさえ決まっていれば、テスト直前時に座標の定数を1度だけ変更すればいい。

 

コーディング

Enumより定数を採用したほうが良い

問題

VBAでは列挙型に文字列を代入することができない。なので、以下の書き方はできない。

Public Enum eAnimals
    Dog = "犬"
    Cat = "猫"
End Enum

 

実際には、PADでは名称で情報を取得し、VBAに取り込むときに列挙型に変換し、「マジックナンバーを削除する」「コード補間を利かせるため」ために利用をしている。

この列挙体を共通モジュールに移動し、文字列から列挙型に変換する、または列挙型から文字列へ変換するメソッドを利用すると以下のようになる。文字数は90文字も使われており、非常に長い。加えて、列挙型のそれぞれについて、変換メソッドを用意するのは結構手間である。

Public Enum eDisplayPoisitonType
    Center = "中央"
    Left = "左"
End Enum

Dim inputValue As Long
inputValue = getCellValue() ' セルから値を取得する
' 値 -> keyへの変換
If (Common.DisplayPositionTypeValueOf(inputValue) = eDisplayPoisitonType.Center) Then

End If
' キー -> 値への変換
If (Common.DisplayPositionTypeStringTo(eDisplayPoisitonType.Center) = inputValue) Then

End If

改善

定数にしたら63文字になった。先ほどの2/3でだいぶ圧縮され、パッと見ただけで何をやっているかが一目でわかるようになった。(DisplayPositionTypeValueが1回しか登場しないのが大きい)

If (Common.DISPLAY_POSITION_TYPE_CENTER = inputValue) Then
End If

大規模な実装であったり、文字列型を設定できる列挙型であれば、当然、列挙型を採用するべきだと思う。

 

貧弱コードは積極的にラップしたほうが良い

問題

VBAはプログラミング言語として貧弱であるため、配列操作、文字列操作等の低レベルな処理に対して行がかさみやすい。

そういった低レベルな処理は本来の業務ロジックと混在させるととにかく可読性が落ちる。

改善

可能な限り、別メソッドなどに切り出す。

例えば、コレクションに対して、distinctを取るといった処理も、共通メソッドに移動させる。

極力低レベルな処理はメソッド呼び出し1行にとどめると、業務ロジックの可読性が驚くほど上がる。

 

実装前に可能な限り、仕様を洗い出し、テストデータは動作確認時に集めておく

問題

  1. 実装前における業務の理解が浅かったため、テスト実施時になって初めて条件分岐の不足に気が付いた。
  2. 実装前にも動作確認を行っていたが、その時に利用したテストデータを保管しておらず、テスト実施時にもう一度テストデータを集めなおすという二度手間が発生した。

改善

  1. 実装前に可能な限り、仕様を洗い出す。仕様書をがっつり作りこむとまでは言わないので、適当な表を作成し、各パラメータの値に応じて、どういった動作をさせるべきかを対応付ける。
  2. テストデータは動作確認時に集めておく、1で作成した表とセットで保管しておくといいだろう。

実装前に必要な機能を洗い出したほうが良い

問題

モジュールに対して機能番号を割り振り、「01_, 02_」としていた。さらに通常の処理モジュールと共通モジュールを分けたほうがいいだろうということで、「C_Common, F01_, F02」に名前を変更した。

さらに、規模が大きく、連番が1つでは耐えられなくなったため、「F01P01, F01P02, F02P01, F02P02….」と連番を2つ用意した。

加えて、連番についても、途中で新しい機能が発生したため、処理の時系列順にするために、モジュール名の以降の連番を1つずつずらすということが2,3度発生した。

改善

まず、連番は10刻みにすべきだった。というのもツールがリリースされた後に機能の追加になったあとに連番をずらすというのは考えられない。普通は間に差し込むべきだ(昔いた会社ではそういった採番をしていたのになぜ忘れていたのか….)

ただし、本質はそこではなく、機能の洗い出しが先に済ませておくのが本来あるべき姿でしょ。

 

リストの構造が後からの変更

問題

ツール上必要なリストの構造がゴロゴロ変わった。これも機能の洗い出しをしていないからだ。

都合、6回ほど、構造が変更になり(項目に対する、セルの列番号が変更になり) 定数の定義を変更しなければいけなくなった。

改善

どんな機能が必要であるかと合わせ、ツール全体を通して、どういった要素が必要になるかを洗い出すべき。

 

変数・メソッド・モジュール名の命名時間と、後からの変更

問題

この記事を書いた目的が、ここにあるといっても過言ではない。

あくまでも記憶ベースであるが、全体132時間の実装時間のうち、変数・メソッド・モジュール名への命名(とりわけ時間のかかったもの)と、後からイケてないことに気が付き、影響調査をしてから、名前の変更を行う。この作業に15時間は割いていたと思う。

明らかに英語に変換しえない単語もその中には確かに存在したのだが、大部分はシンプル命名下手くそ問題なのだと思う。

さらに、いい変数名が思いつかなかったので、いったん仮置きしてロジックの記述を始めたものの大半は実装効率が上がらなかった。歯の間にニラが挟まったような、常にその変数名のアンマッチに違和感を覚えつつで、ロジックのほうに集中できなかった。

一方で、(少なくとも自分の中では)問題ないと思われる変数名が目に入っている間は、ロジックに集中できたのだ。

 

改善

頻出する単語は覚える。単語のみならず、命名の仕方を自分の中で確立する。

 

エクセルからPADの「手動停止」の検知を完全に行うことは無理だという話

問題

エクセルからPADをショートカット経由で起動することは可能であるが、その後のPADの実行状況だとかを把握することはできない。

PADのフロー開始時に「フローを実行しますか」という確認のメッセージでキャンセルを押されたならば、エクセル側でそれを検知できない。また、PADのフローが開始すると、停止ボタンを押すことができる。停止ボタンを押されても、いわゆる、try, catch, finallyのfinallyに当たる後処理のアクションを呼び出すこともできないため、PADからエクセルに処理完了通知を送ることもできない。

改善

PADからセルの値の書き込み、または外部にファイルを出力することで、その内容に応じて「ある程度」、エクセルからPADの状況を把握することはできる。ただそうすると、エクセル側で監視用の無限ループを仕込む必要があり、実装難度が上がる。

リターンに合わないため、エクセル側でPADのコントロールも一元管理するのではなくて、「どうしてもPAD側で停止をしたくなったら、エクセルからでなくPADの停止ボタンを押してね」でよいと思う。

 

良かれと思って追加する機能はコストに見合うのかを考えよう

問題

  1. 別のツールで、ユーザーに対して確認を頻繁に行っており、実行ボタンを1回押しただけで、メッセージボックスが4,5個表示されたことがあった。それを煩わしいと感じたため、ボタンを押した際にメッセージボックスではなくユーザーフォームを表示し、数秒後に、非表示にするようにした。しかし、ユーザー側のほうでメッセージを見落としてしまうことがあるとの声が上がったため、結局メッセージボックスに戻すことにした。
  2. 特殊な処理があり、50件に1件ほど発生するものであるが、追加で必要となる確認作業は30秒 ~ 1分かかる。これをシステムに組み込むとなると、「もともとのマスタでは情報が不足しているため、独自のマスタを用意しなければならない」「もともとのマスタが更新されるたびにメンテナンスを行わなければならない」「導入することで、ツール全体に大きな改修が必要となる」となるため、導入は見送った。
    導入は見送ったものの、導入を検討する際に、コストに見合うかではなく、実装ありきで実装方法のみしか考えられていなかった。

改善

  1. デザイン系についてはモックを作る時点でユーザーの意見を確認したほうがよい。
  2. コストに見合う改修であるかを検討する。

 

要因→改善まとめ

要因 削減余地のあった工数(h)
肩こり 20
職場の騒音 0(無理)
1シートに機能を詰め込みすぎる 3
PADからエクセルへ転記をするためのシートは必ずしも必要ではない 4
レイアウトの体裁を整えすぎる 4
Enumより定数を採用したほうが良い 1.5
貧弱コードは積極的にラップしたほうが良い 3
リストの構造が後からの変更 2
変数・メソッド・モジュール名の命名時間と、後からの変更 12
エクセルからPADの「手動停止」の検知を完全に行うことは無理だという話 4.5
良かれと思って追加する機能はコストに見合うのかを考えよう 6

実装開始時の見積もりは正しかったか

どんぶり勘定であるが、実際にこの程度は削減できたであろう工数が60時間。元々の工数が132時間だったので、132 – 60 = 72時間でおよそ半分ほどになる。

実際には、きちんと仕様を詰める時間が発生するため、さらに時間はかかるであろうが、実装を始める前の「1か月程度で実装できるだろうか」の感覚は概ね誤っていなかったようだ。

改善の優先順位

PADにずいぶん苦しめられた印象があった。それは間違っていないとして、とにかく名前の付け方が下手ということに気付かされた。他人のコードを読む習慣をつけて改善につなげたい。