はじめに
ソースコードを書くとき、可読性や保守性を意識することはとても大切だ。
保守性を高める簡単なアイデアとして、マジックナンバーから定数(Const)への置き換えがある。
新規実装であれば、どのようにコーディングをするか様々なアイデアを巡らせられるが、改修となるとそうもいかない。
次のソースコードを見てください。
三層の負荷分のソースコードです。安心してください、少々内臓がひっくりかえるだけです。
ソースコード
Option Explicit
' csvファイルをインポートする。
Sub LoadFromCsv(filePath As String)
' csvを開く
' Thisworkbookのマスターに張り付ける
' 各項目に対してフォント色や背景色などを書き換える
' csvを閉じる
End Sub
' Errorシートの全量をcsvファイルにエクスポートする。
Sub ExportErrorFile(filePath As String)
' csvに出力する
End Sub
Sub CheckMain()
' csvファイルを読み込む。
Dim filePath As String
filePath = "importPath"
Call LoadFromCsv(filePath)
Dim sh As Worksheet
Set sh = ThisWorkbook.Worksheets("Master")
Dim err_sh As Worksheet
Set err_sh = ThisWorkbook.Worksheets("Error")
' エラー件数
Dim err_count As Long
err_count = 0
Dim i As Long
For i = 1 To 10000
If (sh.Range("A" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("B" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("C" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (CLng(sh.Range("C" & i)) <= 0) Then '年齢が0以下
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (CLng(sh.Range("C" & i)) => 150) Then
'年齢が150以上
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("D" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("E" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("E" & i) <> "男" And sh.Range("E" & i) <> "女" And sh.Range("E" & i) <> "その他") Then
'不正な性別
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("F" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("F" & i) <> "A" And sh.Range("F" & i) <> "B" And sh.Range("F" & i) <> "O" And sh.Range("F" & i) <> "AB") Then
'不正な血液型
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If (sh.Range("G" & i) = "") Then
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If Not (sh.Range("G" & i) Like "080" Or sh.Range("G" & i) Like "090") Then
' 携帯電話ではない
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
If Not (Len(sh.Range("G" & i)) = 11 Or Len(sh.Range("G" & i)) = 12) Then
' 電話番号の桁数ではない
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
End If
Next
' シート(Error)のレコードをすべて出力する。
filePath = "exportPath"
Call ExportToCsv(filePath)
End Sub
ソースコード(仕様)
上記のソースコードに実用性はないし、実行してもまともには動かない。
処理としては以下のような流れである。
- csvファイルからシート「Master」に全量ロード
- 1行ずつ読み込みチェック処理でエラーがあればシート「Error」に1行を転記
- 最後にシート「Error」の全量を入力時と同じ項目の順序で出力
また読み込む/書き込む csvのレイアウトは以下の通り。
- 氏名:A列
- ひらがな:B列
- 年齢:C列
- 生年月日:D列
- 性別:E列
- 血液型:F列
- 電話番号:G列
問題点
いくらかVBAに慣れ親しんでいる方であればこうしたいああしたいといくら案が出てくるはず。
例えば、for文(34行目)で1行分の各列を変数に格納することで以降、Range関数は変数名に置き換わりRangeに渡すアルファベットがなんであるかを気にする必要がなくなるので、可読性が上がり、ミスも減る。
Dim i As Long For i = 1 To 10000 Dim name as String Dim nameKana as String Dim phoneNo as String name = sh.Range("A" & i) nameKana = sh.Range("B" & i) phoneNo = sh.Range("G" & i)
さらに列A~Gのマジックナンバーを定数に置き換えることで列の増減などに対する保守性が上がる。
For i = 1 To 10000
Dim name as String
Dim nameKana as String
Dim phoneNo as String
name = sh.Range(PERSON_COLUMN_DEF_NAME & i)
nameKana = sh.Range(PERSON_COLUMN_DEF_KANA & i)
phoneNo = sh.Range(PERSON_COLUMN_PHONE_NO & i)
また、同じ処理が2回出現したら、関数にしたくなるはずだ。
'未入力エラー
err_count = err_count + 1
err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 7)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 7)).Value
ところで、本記事のタイトルは「初心者向け:コーディング技術を身に付けよう」ではなく「マジックナンバーやめろ」だ。
たしかに修正案1-2でマジックナンバーを定数に置き換えているが、もう少し先がある。本題はそっちだったりする。
具体例として、csvファイルのA列に「マイナンバー番号」が追加されたことを考える。
読み込むcsvファイルは他社から連携されるものであり、変更には口出しできない。
サンプルのソースコードはせいぜい100行足らず。その程度であればどんな可読性・保守性が悪くても読めるし、動く。
そこで、この入力処理が全体で10000行程度であり、単項目チェックに加え非常に複雑な関連チェックが走り、シートもMasterとErrorだけでなく、Master > 中間結果シート1 > 中間結果シート2 … 中間結果シート4 > Error といったようにブック内で転記も何回か行われるものとする。
コード中にセル参照「Range("xxx" & yyy)
」している箇所は数百はある。csvのA列に新しい列が追加されることで、「Range("xxx" & yyy)
」の、列番号「xxx」のアルファベットを1つ後ろにずらすとなると、プログラムをほとんど1からテストしなければいけない。
また、修正案1ー2を採用しても、可読性は上がるが修正量は変わらない。
セル参照から定数名に置き換える過程で、PERSON_COLUMN_DEF_NAME
(名前)とすべきところをPERSON_COLUMN_DEF_KANA
(名前かな)と置き換えてしまうかもしれないので、修正ミスの可能性も変わらない。
それゆえ、中~大規模かつセル参照にてマジックナンバーが多用された既存コードに対して修正を加えるのは実質不可能だ。
改善案
「コードの改修は不可能だ」とは、あくまでも「チェック処理」に対して改修を掛けようとした場合だ。
ファイルの読み込み、書き込み処理に対して手を加えることで既存コードにほぼ影響なく改修することができる。
' 列名定義
Public Const PERSON_COLUMN_DEF_NAME = "氏名"
Public Const PERSON_COLUMN_DEF_NAME_KANA = "氏名(ひらがな)"
Public Const PERSON_COLUMN_DEF_AGE = "年齢"
Public Const PERSON_COLUMN_DEF_BIRTHDAY = "生年月日"
Public Const PERSON_COLUMN_DEF_SEX = "性別"
Public Const PERSON_COLUMN_DEF_BLOOD_TYPE = "血液型"
Public Const PERSON_COLUMN_DEF_PHONE_NO = "電話番号"
Public Const PERSON_COLUMN_DEF_MY_NUMBER = "マイナンバー番号"
' ヘッダー名のリストを取得する。(csvファイルにおける順序)
Public Function GetPersonHeaderByOriginalOrder() As Collection
Dim c As Collection
Set c = New Collection
Call c.Add(PERSON_COLUMN_DEF_MY_NUMBER)
Call c.Add(PERSON_COLUMN_DEF_NAME)
Call c.Add(PERSON_COLUMN_DEF_NAME_KANA)
Call c.Add(PERSON_COLUMN_DEF_AGE)
Call c.Add(PERSON_COLUMN_DEF_BIRTHDAY)
Call c.Add(PERSON_COLUMN_DEF_SEX)
Call c.Add(PERSON_COLUMN_DEF_BLOOD_TYPE)
Call c.Add(PERSON_COLUMN_DEF_PHONE_NO)
Set GetPersonHeaderByOriginalOrder = c
End Function
' ヘッダー名のリストを取得する。(チェック処理における順序)
Public Function GetPersonHeaderByProcessedOrder() As Collection
Dim c As Collection
Set c = New Collection
Call c.Add(PERSON_COLUMN_DEF_NAME)
Call c.Add(PERSON_COLUMN_DEF_NAME_KANA)
Call c.Add(PERSON_COLUMN_DEF_AGE)
Call c.Add(PERSON_COLUMN_DEF_BIRTHDAY)
Call c.Add(PERSON_COLUMN_DEF_SEX)
Call c.Add(PERSON_COLUMN_DEF_BLOOD_TYPE)
Call c.Add(PERSON_COLUMN_DEF_PHONE_NO)
Call c.Add(PERSON_COLUMN_DEF_MY_NUMBER)
Set GetPersonHeaderByProcessedOrder = c
End Function
' csvファイルをインポートする。
Sub LoadFromCsv()
' csvを開く
' Thisworkbookのマスターに張り付ける
' 各項目に対してフォント色や背景色などを書き換える
' csvを閉じる
Dim sh As Worksheet
Set sh = ThisWorkbook.Worksheets("Master")
Dim personHeader As Collection
Set personHeader = GetPersonHeaderByProcessedOrder()
' 「チェック処理」用に項目を並び替える。
Call SortPersonHeaderOrder(sh, personHeader)
End Sub
' Errorシートの全量をcsvファイルにエクスポートする。
Sub ExportErrorFile()
Dim sh As Worksheet
Set sh = ThisWorkbook.Worksheets("Master")
Dim personHeader As Collection
Set personHeader = GetPersonHeaderByOriginalOrder()
' csvファイルの元の項目に並び替える。
Call SortPersonHeaderOrder(sh, personHeader)
' csvに出力する
End Sub
' テーブルを指定した項目順に並び変える。
' order : 項目名のリスト
Private Sub SortPersonHeaderOrder(sh As Worksheet, personHeader As Collection)
Dim index As Long
Dim i As Long
For i = 1 To personHeader.Count
Dim j As Long
For j = 1 To personHeader.Count
' 項目名が一致
If (sh.Cells(1, j) = personHeader(i)) Then
index = j
Exit For
End If
Next
If (i <> index) Then
sh.Columns(index).Cut
Columns(i).Insert
End If
Next
Application.CutCopyMode = False
End Sub
イメージ図
考え方としては非常に単純で、ファイルを読み込んだ時点で、A列の「マイナンバー番号」を末端列に移動させる。チェック処理が全て終了し、csvファイルにエクスポートするタイミングで末端列に移動した「マイナンバー番号」をA列に戻せばよい。
一時的に末端列に移動させることで、A列からG列の項目の並びは「マイナンバー番号」が追加される前と
変わらないためSub CheckMain()
における「Range("xxx" & yyy)
」の列番号は一切修正する必要がなくなった。
当然”G”列を”A”列に挿入するなどとマジックナンバーを使ってはいけない。
提示したコードでは、GetPersonHeaderByOriginalOrder()
とGetPersonHeaderByProcessedOrder()
内のコレクションに追加される順序を変更するだけで、表示順序を好きなように並び替えることができる。ただ、表示順を明示的に数字で示していないため、Dictionaryに置き換えて、dic.Add(項目名,表示順序)
としたほうが分かりやすいかもしれないが、好みの範疇で収まる問題だと思う。
残念なことにエラー出力処理については修正を加えなければいけない。項目数が「7」から「8」に増えたので、err_sh.Range(sh.Cells(err_count, 1), sh.Cells(err_count, 8)).Value = sh.Range(sh.Cells(i, 1), sh.Cells(i, 8)).Value
としなければいけない。もちろん8は何かしらの定数を割り当ててあげるのが良い。
まとめ
マジックナンバーを使うのはやめましょう。