作ったマクロに不備があるかどうか分かりません

質問

様々なサイトを参考にして、自分でマクロを作り、うまく動作したまではいいのですが、不勉強で不備があるかどうか自分では判断できません…。
どなたか不備を指摘してはいただけないでしょうか。

内容：指定したフォルダ内のフォルダ名を、一行ずつ間を空けて書き出す。

Public Sub make_FolderList_02()

' FileSystemObjectを作成する。
Dim Obj As Object
Set Obj = CreateObject("Scripting.FileSystemObject")

' フォルダ名をシート上に出力する為のカウンタ
Dim cnt As Long
cnt = 0

' 「C:\Sample」フォルダ配下に存在するフォルダを一つずつ参照する。
Dim f As Object
For Each f In Obj.GetFolder("C:\Users\test\Desktop\test").SubFolders
' フォルダ名をシート上に出力する。
Range("A1").Offset(cnt, 0).Value = Obj.GetFolder(f).Name
cnt = cnt + 1
' 空白セルを出力する。
Range("A1").Offset(cnt, 0).Value = Null
cnt = cnt + 1
Next f

Set f = Nothing

' 別のフォルダを書き出す

Dim a As Object
For Each a In Obj.GetFolder("C:\Users\test\Desktop\nyannko").SubFolders
' フォルダ名をシート上に出力する。
Range("A1").Offset(cnt, 0).Value = Obj.GetFolder(a).Name
cnt = cnt + 1
Range("A1").Offset(cnt, 0).Value = Null
cnt = cnt + 1
Next a
Set a = Nothing

' オブジェクトを破棄する。
Set Obj = Nothing

End Sub

ここでは、２つのディレクトリを書き出していますが、実際には、６～７つ書き出す予定です。
また、参照先を共有フォルダにした場合、過剰な負荷が掛からないかどうかも、教えていただけると嬉しいです。

大変無駄の多い内容だとは思いますが、ご指導のほどよろしくお願いいたします。

回答No.6

kkkkkm · ベストアンサー

No2です。

書き忘れてましたが以下の部分
Range("A1").Offset(cnt, 0).Value = Obj.GetFolder(f).Name

Offsetを利用してますが、何か特別に理由がなければ(以下ご存知かもしれませんが)

処理速度の速い順(といっても、操作を100万回実施して秒単位での差だったりするようですが)
A列指定の場合
Cells(cnt, 1).Value←右側の引数は、1はA列、2はB列というように列を数値で表します。変数でもOKです。
Cells(cnt, "A").Value
Range("A" & cnt).Value←この書き方は悪評を買ったりしてます。

といった感じでOffsetを利用しなくてもセル指定に変数が使えます。

ただし、最初のcnt = 0はcnt = 1にしておかないとエラーになります。

お礼

業務に使うために独学で覚えた知識なので、処理速度の良し悪しなどは知りませんでした。色々なアプローチの仕方があるんですね。
kkkkkmさんの一連の投稿、大変勉強になりました。ありがとうございました。

回答No.5

noname#223464

要件は満たしているかと思われるので、コード自体には特に不備はないかと思います。
そもそもロジックに正解は無いので、やりたいことが満たされていれば問題ないかと思います。
（重箱の隅を突くような言い方をすれば、エラー処理が無いとか色々あるかもしれませんが。本筋とは関係ないので、これはおいといて。（笑））
ということで、仕様の面から２つほど気になった点を。

（１）
本マクロをどのように実行するのか不明ですが、
仮に、書き出し先シートが確定しているのであれば、Rangeの親オブジェクトのシート名を指定した方が意図しない動作をする可能性は低くなります。
ボタンやメニュー等から実行し、アクティブシートに書き出すのであれば、特に気にする必要は無いかと思います。

（２）
書き出し先シートが常にクリアされているか不明ですが、
仮に、既に値の入ったシートに書き出すのであれば、書き出し先シートのA列を最初にクリアする処理を入れといた方が意図しない出力をする可能性は低くなります。
書き出し先シートのA列が常にクリアされている状態であれば、特に気にする必要は無いかと思います。

以上です。

お礼

明らかな不備はないということで、安心しました。
ご指摘いただいた中で、（２）については思い至っていなかったので、追加いたしました。回答ありがとうございます。

回答No.4

kkkkkm

No2,No3です

UBound(RootFolder)

にした場合(探すディレクトリに増減があった場合の事を考えると)

Dim RootFolder(1) As String
↓
Dim RootFolder() As String

RootFolder(0) = "C:\Users\Test"
RootFolder(1) = "C:\Users\Test2"
↓
ReDim RootFolder(0)
RootFolder(0) = "C:\Users\Test"
ReDim Preserve RootFolder(1)
RootFolder(1) = "C:\Users\Test2"

にそれぞれ変更しておいた方が安全です。

お礼

ご丁寧に、回答していただきありがとうございます。
教えていただいたやり方だと、コードが短くてすみそうです。特に、 空白セルについてはすぐに修正したいと思いました。参考にして、色々やってみます。

回答No.3

kkkkkm

No2です

FolderCount = 1 'RootFolderの数値に合わせます。
↓
FolderCount = UBound(RootFolder)

もしくはFolderCountをなくしてしまって

Call kakidasi(MyObj, RootFolder, cnt, FolderCount)
↓
Call kakidasi(MyObj, RootFolder, cnt, UBound(RootFolder))

に訂正です。

回答No.2

kkkkkm

おなじコードを何回も書くのは面倒なので、同一部分を別のプロシージャにしてメインから呼び出すようにしておくと楽です。あまり美しくないけど以下のような感じでいかがですか。

Call kakidasiで下のPrivate Sub kakidasiを呼び出しています。
探すディレクトリが増えたらRootFolder(x)を必要分確保すればそれだけで修正が済みます。

Public Sub make_FolderList_02()
Dim MyObj As Object
Dim cnt As Long, FolderCount As Integer
Dim RootFolder(1) As String

Set MyObj = CreateObject("Scripting.FileSystemObject")

RootFolder(0) = "C:\Users\Test"
RootFolder(1) = "C:\Users\Test2"

cnt = 0
FolderCount = 1 'RootFolderの数値に合わせます。

Call kakidasi(MyObj, RootFolder, cnt, FolderCount)

Set MyObj = Nothing

End Sub

Private Sub kakidasi(MyObj As Object, FoldersName() As String, ByRef cnt As Long, ByVal FolderCount As Integer)
Dim f As Object
Dim i As Integer

For i = 0 To FolderCount
For Each f In MyObj.GetFolder(FoldersName(i)).SubFolders
Range("A1").Offset(cnt, 0).Value = MyObj.GetFolder(f).Name
cnt = cnt + 1
' 空白セルを出力する。もともと空白なら上の行でcnt = cnt + 2としておけば下の2行はいりません
Range("A1").Offset(cnt, 0).ClearComments
cnt = cnt + 1
Next f
Set f = Nothing
Next i

End Sub

回答No.1

bin-chan

不備とまでは言えないが、２点気になりました

コメント行、残すならきちんとした内容に。

前半と後半の差は、FSOに渡す対象フォルダでしょ？
だったらくくり出して、引数で対象フォルダと行カウンタ（ｃｍｔ）を
渡せば、6～7回の繰り返しを作るのは楽になると思いますよ

お礼

コメント行、人に見せることを考慮して書き直します。
カウンタについては、すぐにはできそうにありませんが、他の方の回答も参考にして修正してみます。早速の回答ありがとうございました。