見やすい モジュール,プロシージャ の書き方

Question

見やすい モジュール,プロシージャ の書き方

今まで人に自分のコードを見てもらったことが無かったので
書き方がその時の気分によって変わっていました。

見やすくするために自分なりに条件を決めてみました。
悪いところ等の指摘をお願い致します。

決めてみた条件

• 変数の宣言を強制Option Explicitを入れ一行空ける

• 先ず有れば条件付きコンパイルを書く

• 「PrivateよりもPublicの方が上」の様にスコープが広いものほど上に書く

• 定数より変数を上に書く

• 宣言は同じ型が隣り合うように、またカウンタ変数など役割が同じものは隣り合うようにする。

• 宣言の後は一行空ける

• プロシージャの前後は3行空ける

• プロシージャ内の最初の行に処理の簡易説明、その次に 返値と引数の補足説明

• 宣言は インデントは1

• 処理は インデントは2

• ループ等の始まりと終わりがセットになっているものは その中に書く処理はインデントを一つ増やす

• 但し配列処理などの多重ループはあえてインデントをそろえる

• 処理のグループはインデント数が同じ文章と隣り合う場合は一行空ける

条件に沿って過去に書いたコードを手直ししてみました。

Option Explicit

'配布する時には以下のフラグをFalseにする
#Const Coding = True
#If Coding Then
    Private FSO As FileSystemObject
    Private TextStream_ As TextStream
#Else
    Private FSO As Object
    Private TextStream_ As Object
#End If
#If VBA7 And Win64 Then '64ビット版
    Private Declare PtrSafe Function QueryPerformanceFrequency Lib "kernel32" (frequency As Double) As Long
    Private Declare PtrSafe Function QueryPerformanceCounter Lib "kernel32" (procTime As Double) As Long
#Else '32ビット版
    private Declare Function QueryPerformanceFrequency Lib "kernel32" (frequency As Double) As Long
    private Declare Function QueryPerformanceCounter Lib "kernel32" (procTime As Double) As Long
#End If

Private PNo As Long
Private Indent As Long
Private StartTime As Double
Private ErrorFlag As Boolean
Private FIlename As String
Private Const IndentWide As Long = 2



Function Redims(ByVal Ar As Variant, Optional ByVal Vertical As Long = -1, Optional ByVal Horizontal As Long = -1) As Variant
'二次元配列のリサイズを行う
'返値:二次元配列
'    Base は 0
'引数1: Ar = 二次元配列
'    Base は 0 or 1
'引数2: Vertical = 縦の要素数
'    Base 0基準
'引数3: Horizontal = 縦の要素数
'    Base 0基準

    Dim min As Long
    Dim VC As Long
    Dim HC As Long
    Dim VCF As Boolean
    Dim X As Long
    Dim Y As Long
    Dim Tray As Variant

On Error GoTo Err
        '元の配列サイズ
        min = LBound(Ar)
        VC = UBound(Ar)
        HC = UBound(Ar, 2)
        
        '変更後の配列サイズが指定されていなければ保持
        If Horizontal = -1 Then
            Horizontal = HC - min
        End If
        If Vertical = -1 Then
            Vertical = VC - min
        End If
        
        '配列の移し替え
        ReDim Tray(Vertical, Horizontal)
        Do While (Y <= Vertical And Y + min <= VC)
        Do While (X <= Horizontal And X + min <= HC)
            Tray(Y, X) = Ar(Y + min, X + min)
            X = X + 1
        Loop
            X = 0
            Y = Y + 1
        Loop
        
        '返値の指定
        Redims = Tray
Err:
End Function



Function Extract(ByVal Ar As Variant, ByVal Index As Long, Optional ByVal Vertical As Boolean) As Variant
'二次元配列から任意の一行(列)を一次元配列で抜き出す
'返値:一次元配列
'    Base は 0
'引数1: Ar = 二次元配列
'    Base は 0 or 1
'引数2: Index = 抜き出す行又は列
'    Ar の Base 基準
'引数3: Vertical = 抜き出す方向
'    True:縦方向
'    False:横方向

    Dim Ans As Variant
    Dim Str As String
    
        With WorksheetFunction
            '列を書き出す場合は配列の縦横を反転
            If Vertical Then
                Ar = .Transpose(Ar)
            End If
            
            '指定された行をタブ区切りの文字列として格納
            Str = Join(.Index(Ar, Index, 0), vbTab)
        End With
        
        'タブ区切りの文字列を一次配列に変換
        Ans = Split(Str, vbTab)
        
        '返値の指定
        Extract = Ans
End Function

具体的な処理の内容は置いておいて
見やすさ はどうでしょうか？
ご教授をお願い致します。

蛇足 見やすいコードが書きたい

最近 作る内容が複雑になってきて過去に作ったコードを再利用出来る様に
したいと思い始めました。
現状　コードがぐちゃぐちゃで探すよりも書き直した方が早いという感じで
再利用が出来ません。
書籍を買って読んだりしていますが
知識が増える度に選択肢の多さに迷い
手が止まります。
最終目標は判断基準の確立と
コードの使いまわしが出来るようにすることです。
宜しくお願い致します。

Comments

[m.ts10806]

(後で見やすいコードの書き方が分からない_2)　は不要かと思います。タイトルは要件のみを記載してください。

[kamikazelight]

失礼致しました。

[m.ts10806]

質問は編集できるので指摘があった場合は適宜対応してください。

[kamikazelight]

すみませんでした。タイトルも修正できることを初めて知りました。気を付けます。

Answer 1

いい質問だね。

折角だから、自分のマイルールを少しばかり。

・Option Private Module を Option Explicit の下に追記する
※ユーザ側から関数を隠す

・関数の説明
中でも外でもどちらでもメリデメがあると思います。

中に書く場合は、VBAでコードの説明を抜き出す時に便利
(モジュール内の最初の関数の説明を抜き出すのが面倒)
また、１関数だけ表示する「プロシージャの表示」にしても説明が見れる
(外に書いた場合は説明が見えない)

　ただ、往々にして、説明が更新されずに処理が変わる、という事が起こりやすいので、
説明を書くよりも、コードを分かりやすく書く方が重要だと思います。
分かりやすく書くという事は、関数名、変数名、処理の流れ等を意識するという事ですね。

・変数の宣言位置
最初は関数の頭に書くように教えられたけど、
正直、コーディングしにくい。。。
しかも、他の言語では直前に宣言する事が主流だと聞く。
なので、自分も必要になった時に、近場の分かりやすい位置に書くようにしてます。

・繰り返し
どうしても Do-Loop じゃなきゃって時意外は、For-Next がメインかな。
ていうか、インクリメントを自分で書くという事は、バグの可能性が増えるということで。。。
そして、Do-Loop は無限ループの可能性があるから、敬遠する気持ちが必要かと。。。

あと、ループｘ２の時にインデント揃えるのは微妙な気がする。。。

・ネーミングセンス
これからビシバシ鍛えた方がいいと思うよ。
Ar なんていう変数名、何だと思えばいいのか。。。
共通関数ならなおさら。

他にも２文字を多用してるけど、なぞなぞになっちゃうのでは？　

Vertical って変数名で Boolean型 にしてるところは、
自分は Direction って変数名で xlRowCol列挙体 を使うかな～
というのも、エクセルではシートの行・列操作が多いから、
必然的に配列も１，２次元配列で十分なんだよね。
だから、配列の次元数は、基本的に２次元以内という前提で共通関数を作ってるよ。

・関数名
色々なんだよね。まぁ現場に合わせるのが原則だけど。
ただ、自分ルールでは、共通関数は fArray_Trans とか、
頭にf、その後にArray とか String とか Range とかメインとなる名称、
で、その後ろに機能を付けていくようにしてます。

非常に多くの共通関数を作っていくうちに、こうなったんだけど、
理由としては、自作関数だと見て分かりやすい事と、
f〇〇でインテリセンスを出せば、名前を憶えてなくてもリストから見て探せる事。

※Array用の共通関数は、それ用の１モジュールにまとめてたりします(M_Array モジュール等)

※本当はクラス化とかした方がいいのかもだけど、VBAの世界でクラス化を進め過ぎると、
ついてこれない人がいるかもだし、移植しやすいので、今はこれで良しとしてます。

※要望としては既存のオブジェクトの機能を上書きしたいんだけど。。。VBAではできないから残念。。。

Comments

[kamikazelight]

Option Private Module 初めて知りました。
クラスにするかプライベートにしないと
隠せないと思っていました。
必ず入れるようにします。

xlRowColは調べて始めて知りましたが
自動補完が利かないので
忘れた時にどんな値を指定していいのかわからなくなりそうですね...

また 宜しくお願いします

[ExcelVBAer]

自動補完って、インテリセンスのことかな？
Excelなら、xlRows か xlColumns を選べるはずなんだけど。。。

[kamikazelight]

出てこないですね
Sub test()
Dim RowCol As XlRowCol
RowCol.
End Sub

VBEに入力してピリオドまで打った状態でCtrl+Space 押しても
候補が出てきません。

ないとは思いますが環境の違いでしょうか?
こちらの環境
Excel 2016 64bit

[ExcelVBAer]

えと、、、列挙体を使ったことがなかったのかな？
自分で Enum を作っても同じ事になるんだけど、
この場合だと、「RowCol = 」の状態で候補が出てくるはず。

どう伝えるのが正しいか分からないけど、
Boolean型 は True/False が候補に出てくるように、
XlRowCol型 は xlRows/xlColumns が候補になる、という感じ。

[kamikazelight]

ありがとうございます...
列挙体は一年以上前に「へーこんな機能があるんだー」っと使ったきりで
完全に忘れてました。
思い出しました...

[ExcelVBAer]

僕はよく使ってるよ～
特に、シートの行や列を定義するのに超便利！
↓の fRange_Value と Enum の組合せは最強だと思ってるので、お試しあれ～
http://www.excelsystem.jp/Library/archives/3

Answer 2

前回も書きましたが、書いてる人が見やすければそれでよしです。
但し、条件のうち以下の4つはあまり一般的ではないと思われます。

プロシージャの前後は3行空ける
プロシージャ内の最初の行に処理の簡易説明、その次に 返値と引数の補足説明

これは2つセットで考えますが、関数の説明は一般的に外に置きます。
そうすることで関数ごとの区切りがわかりやすくなります。
またそうすることで「3行空ける」などの縛りも必要なくなります。
関数の説明文もなるべく簡潔に書いたほうが良いでしょう。

処理は インデントは2

無用なインデントは避けるべきです。
基本的に関数内は変数宣言以降処理しかないはずですから（途中で変数宣言をする場合を除く）そうすると無駄にインデントが空いた状態になります。
プログラムは右へ行くほど見づらくなりますので、必要のないインデントは行わないべきです。

但し配列処理などの多重ループはあえてインデントをそろえる

ループの開始と終わりが簡単に認識できるようにするためにもこれは行わないべきです。

VBAの場合、自動的にコード整形が行われるので、それにそぐわない書き方をすると生産が落ちる可能性があります。
自動でここから書きなさいとインデントしてくれているのに、更にインデントを加えたり、また逆に減らしたりといったことです。

というのを踏まえて前回同様私なりに修正したコードを貼っておきます。
（関数の説明は実際にそろっているのですが、この解答欄ではずれてしまいます）

Option Explicit

'配布する時には以下のフラグをFalseにする
#Const Coding = True

#If Coding Then
	Private FSO As FileSystemObject
	Private TextStream_ As TextStream
#Else
	Private FSO As Object
	Private TextStream_ As Object
#End If
#If VBA7 And Win64 Then '64ビット版
	Private Declare PtrSafe Function QueryPerformanceFrequency Lib "kernel32" (frequency As Double) As Long
	Private Declare PtrSafe Function QueryPerformanceCounter Lib "kernel32" (procTime As Double) As Long
#Else '32ビット版
	private Declare Function QueryPerformanceFrequency Lib "kernel32" (frequency As Double) As Long
	private Declare Function QueryPerformanceCounter Lib "kernel32" (procTime As Double) As Long
#End If

Private PNo As Long
Private Indent As Long
Private StartTime As Double
Private ErrorFlag As Boolean
Private FIlename As String
Private Const IndentWide As Long = 2

'二次元配列のリサイズを行う
'	返値		: 二次元配列 Base は 0
'	Ar		: 二次元配列 Base は 0 or 1
'	Vertical	: 縦の要素数 Base 0基準
'	Horizontal	: 縦の要素数 Base 0基準
Function Redims(ByVal Ar As Variant, Optional ByVal Vertical As Long = -1, Optional ByVal Horizontal As Long = -1) As Variant

	Dim min As Long
	Dim VC As Long
	Dim HC As Long
	Dim VCF As Boolean
	Dim X As Long
	Dim Y As Long
	Dim Tray As Variant

	On Error GoTo Err

	'元の配列サイズ
	min = LBound(Ar)
	VC = UBound(Ar)
	HC = UBound(Ar, 2)

	'変更後の配列サイズが指定されていなければ保持
	If Horizontal = -1 Then
		Horizontal = HC - min
	End If
	If Vertical = -1 Then
		Vertical = VC - min
	End If

	'配列の移し替え
	ReDim Tray(Vertical, Horizontal)
	Do While (Y <= Vertical And Y + min <= VC)
		Do While (X <= Horizontal And X + min <= HC)
			Tray(Y, X) = Ar(Y + min, X + min)
			X = X + 1
		Loop
		X = 0
		Y = Y + 1
	Loop

	'返値の指定
	Redims = Tray
Err:
End Function

'二次元配列から任意の一行(列)を一次元配列で抜き出す
'	返値	: 一次元配列 Base は 0
'	Ar	: 二次元配列 Base は 0 or 1
'	Index	: 抜き出す行又は列 Ar の Base 基準
'	Vertical: 抜き出す方向 True=縦方向 False=横方向
Function Extract(ByVal Ar As Variant, ByVal Index As Long, Optional ByVal Vertical As Boolean) As Variant

	Dim Ans As Variant
	Dim Str As String

	With WorksheetFunction
		'列を書き出す場合は配列の縦横を反転
		If Vertical Then
			Ar = .Transpose(Ar)
		End If

		'指定された行をタブ区切りの文字列として格納
		Str = Join(.Index(Ar, Index, 0), vbTab)
	End With

	'タブ区切りの文字列を一次配列に変換
	Ans = Split(Str, vbTab)

	'返値の指定
	Extract = Ans
End Function

Comments

[kamikazelight]

On Errorのインデントですが
もしも 後ろで On Error Goto 0 等を
入れる場合はエラー処理対象の始まりと
終わりの境目が見にくくなる気がしますが、
気にしない感じでしょうか
それともOn Errorインデントを減らしたり、後の処理のインデントを増やしたりするのでしょうか

Do Whileですが
もし、三次配列ループ内でIfを使うと
かなり階層が深くなりますが
そういう場合は気にしない感じでしょうか？
それとも、別関数として書き出したりするのでしょうか。
教えて下さい。

[ttyp03]

On Errorのインデントについては私もどちらがよいと言えないですね。
私も昔は1文字目から書いてましたけど、最近は特に気にしていないです（というかまじめにVBAのコードは書いていない）。

一般的に（明確な理由もなく）ネストが3重にもなるのは何か設計上の問題があると思います。
三次元配列のような明確な理由がある場合は仕方ないでしょう。
但し最深の処理でIfしてSelectしてみたいに更なるネストを生むようであれば、関数化するなりしたほうがよいかもしれません。
但し安易に関数化すると可読性が落ちる可能性もあるので、実際のコードを見て判断するしかないです。

[kamikazelight]

さじ加減が難しいですね...
頑張ります。

Answer 3

コードを見て気になる点

・Functionの説明がFunction内に記述されている。Function定義の前に移動した方がいいと思います。

・２つのDo Whileのインデントが同じ位置なのが気になります。２つ目のDo Whileはインデントするべきでは？

・Function ExtractのDimのインデントとその後のWith以降のインデントが違和感あります。DimとWithは同じ位置がいいと思います。

・On Error GoTo Errの位置も上のDimと合わせるかな。

・'変更後の配列サイズが指定されていなければ保持の下のIF～End Ifと次のIf～End Ifの間は１行空けますね私なら

上記はあくまで私の私見なので自分が見やすいのが一番だと思います。

Comments

[kamikazelight]

試しに直してみたら見やすくなりました。

ただ、
あんまりないですが Withのネストをするときは
内側のWithもDimのラインでいいでしょうか

Do Whileですが
三次配列以上(三次配列はちょくちょくありますが、四次配列以降は使ったことないです。)の場合
インデント数が多くなってしまうので
・ループ等の始まりと終わりがセットになっているものは その中に書く処理はインデントを一つ増やす
・但し配列処理などの多重ループはあえてインデントをそろえる
としていました。
もし、三次配列ループ内でIfを使うと
かなり階層が深くなりますが
そういう場合は気にしない感じでしょうか？
それとも、別関数として書き出したりするのでしょうか。
教えて下さい。

[unz.hori]

ネストする場合はインデントしますね。
階層が深くなる場合は切り出したりします。ただ、そこでしか使わない場合はそのまま記述します。
見やすくなるからといって切り出すと逆に追いかけにくくなるので。
インデントもTABの場合、エディタ上で変更できれば1TAB = 2SPACEとかで見やすくしたりします。

[kamikazelight]

なるほど...
階層が深くなった場合は他でも使える処理なら書き出し、そうでないなら
インデントの幅を減らして見通しをよくするのですね
参考になります!!

Answer 4

コードを読んで気になった点。
a. 以下の変数の用途が書いていないのが気になりました。
何のための変数なのでしょうか。

Private PNo As Long
Private Indent As Long
Private StartTime As Double
Private ErrorFlag As Boolean
Private FIlename As String
Private Const IndentWide As Long = 2

b. 以下はWinAPIの宣言ですよね。宣言をPrivateからPublicにして別モジュールにすることをお勧めします。

#If VBA7 And Win64 Then '64ビット版
    Private Declare PtrSafe Function QueryPerformanceFrequency Lib "kernel32" (frequency As Double) As Long
    Private Declare PtrSafe Function QueryPerformanceCounter Lib "kernel32" (procTime As Double) As Long
#Else '32ビット版
    private Declare Function QueryPerformanceFrequency Lib "kernel32" (frequency As Double) As Long
    private Declare Function QueryPerformanceCounter Lib "kernel32" (procTime As Double) As Long
#End If

インデントや見やすさは勿論大事なのですが、
調査のために3ヶ月後に処理を読んだ時に思考が遮らないような
モジュール分けやコメントも意識してみてはどうでしょうか。

Comments

[kamikazelight]

調査のために3ヶ月後に処理を読んだ時に思考が遮らないような を
実現するために見やすくしたいと思っております
WinAPIをprivateにしているのは移植の時にAPI定義のしわすれやAPI定義かぶるのを
嫌った為ですが確かに分けたほうが良さそうな気がしますね。
考え直してみます

モジュール分けなども今困ってます...
別質問で出す予定でいますので
またよろしくお願い致します