VBAのコードの保守性を高めたい【高速化、変数化、エラートラップ】

https://qiita.com/SoreKiita/items/3d2e6f2e573818d11e0b
qitaにあったスクショ自動貼り付けのコードについて質問です。

Declare Function OpenClipboard Lib "user32" (Optional ByVal hwnd As Long = 0) As Long
Declare Function CloseClipboard Lib "user32" () As Long
Declare Function EmptyClipboard Lib "user32" () As Long
Public LOCALOFFSET As Long

Sub AutoCapture()

'クリップボードを空にする。
OpenClipboard
EmptyClipboard
CloseClipboard

MsgBox "AutoCaptureを開始します。" & vbNewLine & _
    "①貼り付け開始位置にカーソルを置いて" & vbCr & _
    "②貼り付け開始位置変える場合はリセットボタン押して" & vbCr & _
    "③このマクロ起動中はエビデンスに全て貼付けられるから注意", vbInformation
Dim CB As Variant
Dim IsFirst As Boolean: IsFirst = True
Do While True
    CB = Application.ClipboardFormats
    If StrConv(ActiveSheet.Cells(1, 1).Value, vbUpperCase) = "EXIT" Then GoTo Quit
    If CB(1) <> -1 Then
        For i = 1 To UBound(CB)
            If CB(i) = xlClipboardFormatBitmap Then
                If IsFirst = True Then
                    ActiveSheet.Paste Destination:=ActiveCell
                    IsFirst = False
                    LOCALOFFSET = LOCALOFFSET + CInt(Sheets("メニュー").Range("kankaku").Value)
                Else
                    ActiveSheet.Paste Destination:=ActiveCell.Offset(LOCALOFFSET, 0)
                    LOCALOFFSET = LOCALOFFSET + CInt(Sheets("メニュー").Range("kankaku").Value)
                End If

                'クリップボードを空にする。
                OpenClipboard
                EmptyClipboard
                CloseClipboard
            End If
        Next i
    End If
    DoEvents
Loop

Quit:
MsgBox "AutoCaptureを停止しました。", vbInformation
ActiveSheet.Cells(1, 1).ClearContents
End Sub

保守性を高める為に同じ処理を行う部分は変数化したいです。
例えば、上記のクリップボードを空にするコードを変数化したいですが、可能でしょうか？

また、変数宣言に関してですが、最上段に宣言と処理は書いた方が良いですよね？見やすいコードにしたいです。

エラートラップでgotoを使っているのですが、gotoは良くないと聞いたので別のやり方を教えて頂きたいです。

vbaに関して実務レベルで使えるような勉強ができるサイトや本など有れば教えて頂きたいです。
基礎の勉強は行ったのですが、それだけだとコード一つ一つの意味が分からないです。
皆さんはどうやって覚えたのでしょうか？

回答ではなく、調べ方を教えて頂けるだけでもありがたいです。よろしくお願いします！

ttyp03

2020/06/04 23:56

コードが見づらいのでインデントを調整したうえでコードタグで括ってください。

dodox86

2020/06/05 00:09

他のサイト、更に他の方が書いたコードを勝手に貼り付けた上で、「このコードはこうした方が良いですよね？」とはずいぶん失礼な話だと思うのですが、どうでしょうか。

退会済みユーザー

2020/06/05 03:29

>>dodox86 引用って知ってますか？指摘のポイントがズレています。他人のコードを改善したいのではなく、自分の、保守性、可読性を高めるコードを書く能力を高めたくて質問しています。このサイトはpgとしての能力を高める為の場だと考えていますがいかがですか？

行動規範の内容に同意します

回答3件

ベストアンサー

たぶん色々勘違いしていると思います。

保守性を高める為に同じ処理を行う部分は変数化したいです。
例えば、上記のクリップボードを空にするコードを変数化したいですが、可能でしょうか？

変数化ではなく関数化ですよね？
可能です。
クリップボードの処理では戻り値が不要なのでSub関数で良いでしょう。

また、変数宣言に関してですが、最上段に宣言と処理は書いた方が良いですよね？

どこに宣言するかは主観が大きいのでご自分が見やすいと思う方法でよいです。
私は関数内先頭で宣言する派です。
もっとも業務用途でコーディング規約があるならそれに従ってください。

エラートラップでgotoを使っているのですが、gotoは良くないと聞いたので別のやり方を教えて頂きたいです。

このコードにあるGOTOの処理はエラートラップではなく、単なる終了判定です。
別にGOTOでも構わないと思いますが、このコードの場合であればループを抜ければよいので、Exit Do で良いと思います。

vbaに関して実務レベルで使えるような勉強ができるサイトや本など有れば教えて頂きたいです。
基礎の勉強は行ったのですが、それだけだとコード一つ一つの意味が分からないです。
皆さんはどうやって覚えたのでしょうか？

業務レベルといってもちょっとしたツールからデータベースと連携したりと言った大掛かりなものまで様々です。
勉強＝作ること、だと思うので、作り続けるしかないのではと思います。
本やサイトでいくら学んでもいきなり実践で使えると思えませんので。

ということで、関数化とGOTOをExit Doに変更したコードを置いておきます。

VBA
1Sub ClearClipboard
2	OpenClipboard
3	EmptyClipboard
4	CloseClipboard
5End Sub
6
7Sub AutoCapture()
8
9	'クリップボードを空にする。
10	Call ClearClipboard
11
12	MsgBox "AutoCaptureを開始します。" & vbNewLine & _
13	"①貼り付け開始位置にカーソルを置いて" & vbCr & _
14	"②貼り付け開始位置変える場合はリセットボタン押して" & vbCr & _
15	"③このマクロ起動中はエビデンスに全て貼付けられるから注意", vbInformation
16	Dim CB As Variant
17	Dim IsFirst As Boolean: IsFirst = True
18	Do While True
19		CB = Application.ClipboardFormats
20		If StrConv(ActiveSheet.Cells(1, 1).Value, vbUpperCase) = "EXIT" Then Exit Do
21		If CB(1) <> -1 Then
22			For i = 1 To UBound(CB)
23				If CB(i) = xlClipboardFormatBitmap Then
24					If IsFirst = True Then
25						ActiveSheet.Paste Destination:=ActiveCell
26						IsFirst = False
27						LOCALOFFSET = LOCALOFFSET + CInt(Sheets("メニュー").Range("kankaku").Value)
28					Else
29						ActiveSheet.Paste Destination:=ActiveCell.Offset(LOCALOFFSET, 0)
30						LOCALOFFSET = LOCALOFFSET + CInt(Sheets("メニュー").Range("kankaku").Value)
31					End If
32
33					'クリップボードを空にする。
34					Call ClearClipboard
35				End If
36			Next i
37		End If
38		DoEvents
39	Loop
40
41	MsgBox "AutoCaptureを停止しました。", vbInformation
42	ActiveSheet.Cells(1, 1).ClearContents
43End Sub

投稿2020/06/05 00:09

ttyp03

総合スコア17000

退会済みユーザー

2020/06/05 09:28

ご回答頂きありがとうございました！ callを使って、プロシージャを分けることを知らなかったので大変参考になりました。要領の得ない質問だったかもしれず申し訳ありません。今回、回答頂いた事で、プロシージャを分け、ステートメント毎になにが書かれているかを理解することができたので非常に自分の能力の伸びを実感できました。たしかに、実際に作るということが一番勉強になりますね。色々自分で作ってみて、能力を伸ばしていきます。書いて頂いたコード大変参考になりました。ありがとうございました????

行動規範の内容に同意します