説明に一致するフォルダーとワークブックをセットアップしました。以下の修正されたコードは私にとってはうまくいきます。元のコードがすべてあなたのものであることを願っています。いくつかの間違いや悪い習慣について初心者を簡単に許すことができますが、あなたが提案するようにチュートリアルからこれを得た場合、私は愕然とします.
すべての変更を説明するコメントを含めました。必要に応じてさらに説明を求めて戻ってきてください。ただし、私のコードを自分で解読すればするほど、スキルの開発が早くなります。コードが何を行っているかを示すコメントを含めますが、ステートメント自体に関するコメントはほとんどありません。たとえば、Option Explicit存在を知っていれば簡単に検索できるため、その目的は説明されていません。
Option Explicit ' Always have this statement at the top of every module
' Constants are fixed while a macro is running but can be changed
' if the data is redesigned. This defines the first data row of every
' worksheet is 2. That is, it allows for one header row. I could have used
' 2 within the code below. If you ever have to update code because the
' number of header rows has changed or a new column has been inserted in
' the middle of existing columns, you will understand why I use constants.
Const RowDataFirst As Long = 2 ' Set to 1 if no header rows
' You assume that when you open a workbook, the active worksheet is the one
' required. This is only reliable if the workbooks only have one worksheet.
' I have defined one constant for the name of the worksheet within the
' destination workbook and one for name of the worksheet within every
' source workbook. I assume this is adequate. I will have alternative
' suggestions if it is not adequate.
Const WshtDestName As String = "Data"
Const WshtSrcName As String = "Data"
Sub copyDataFromMultipleWorkbooksIntoMaster()
Dim FolderPath As String
Dim FilePath As String
Dim FileName As String
Dim HeaderCopied As Boolean
Dim RowDestNext As Long
Dim RngDest As Range
Dim RngSrc As Range
Dim WbkDest As Workbook
Dim WbkSrc As Workbook
Dim WshtDest As Worksheet
Dim WshtSrc As Worksheet
Application.ScreenUpdating = False
' You need row numbers in both the source and the destination worksheets.
' Use names for variables that tell you exactly what the variable is for.
' While you are writing a macro, it is easy to remember odd names but if
' you return to the macro in six or twelve months will you still remember?
' I have a naming system which I always use. I can look at macros I wrote
' ten years ago and know what all the variable are which is an enormous help
' when updating old code. If you do not like my system then develop your own.
' My names consist of a series of keywords with the most global first.
' "Row" says the variable is a row number. "Wbk" says the variable is a
' workbook. "RowXxx" says the variable is a row number within worksheet or
' array Xxxx. "RowSrcXxx" says the variable is a row number for worksheet
' "Source". "Xxx" can be "First", "Crnt", "Next", Prev", "Last" or whatever
' I need for the current macro
Dim RowSrcLast As Long
' My comment suggested you be consistent in your use of column numbers but
' comments do not allow enough room to explain. With some statements, having
' a different number of rows or columns in the source and destination can
' give funny results with truncation or duplication. If you know you only
' want 9 columns then use 9 in both source and destination ranges. If the
' number of columns might change then determine the number at runtime.
Dim ColSrcLast As Long
' If you are handling multiple workbooks be explicit which workbook
' you are addressing. This assumes the workbook into which the worksheets
' are collected is the workbook containing the macro.
Set WbkDest = ThisWorkbook
Set WshtDest = WbkDest.Worksheets(WshtDestName)
' Note a worksheet variable references the worksheet within its workbook.
' I do not need to write WbkDest.WshtDest.
' FolderPath = "C:\Users\PC-1\Desktop\Merge\"
' You can hard code the name of the folder into a macro but it is
' a bother when you move your workbooks. When all your workbooks
' are in the same folder, the following is more convenient
FolderPath = WbkDest.Path & "\"
FilePath = FolderPath & "*.xls*"
' Note Dir searches down the folder index for files that match the template.
' The sequence in which they are found depends on the sequence in which the
' files were added to the folder. There are other techniques if sequence is
' important.
FileName = Dir$(FilePath) ' Dir$ is marginally faster than Dir
' Your existing code adds new data to the end of the existing worksheet in
' Master.xlsm. This may be correct but it is more usual to clear the
' destination at the start of each run. Comment out the first block and uncomment
' the second block if you want to add to existing data.
With WshtDest
.Cells.EntireRow.Delete ' Delete every row in worksheet
HeaderCopied = False ' There is no header within the destination worksheet
RowDestNext = 1 ' First (only) header row will be copied from first
' source worksheet to this row
End With
' If you know that column A of the used rows of the active sheet contains no
' blank cells, the following is the easiest way of finding that last used row:
' RowDestLast = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row
' But this technique is unreliable if there might be blank cells. No technique
' is 100% reliable but you would have very strange data if the technique I have
' used is not reliable for you.
'With WshtDest
' ' Find last row with a value
' Set RngDest = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByRows, xlPrevious)
' If RngDest Is Nothing Then
' ' No data has been found so the worksheet is empty
' HeaderCopied = False ' There is no header within the destination worksheet
' RowDestNext = 1 ' First (only) header row will be copied from first
' ' source worksheet to this row
' Else
' ' There is data within the worksheet. Assume the header row(s) are present.
' HeaderCopied = True
' RowDestNext = RngDest.Row + 1
' End If
'End With
' Please indent your code within Do-Loops, If, etc. It makes your code
' much easier to read.
' All your workbooks are within the same folder. Master.xlsm will be one
' of those found but you do not want to use it as a source workbook.
Do While FileName <> ""
If FileName <> WbkDest.Name Then
Set WbkSrc = Workbooks.Open(FolderPath & FileName)
' WbkSrc will be the active workbook but better to reference it explicitly
With WbkSrc
Set WshtSrc = .Worksheets(WshtSrcName)
End With
With WshtSrc
' Find last row with data if any
Set RngSrc = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByRows, xlPrevious)
If RngSrc Is Nothing Then
' No data has been found so the worksheet is empty
Else
RowSrcLast = RngSrc.Row
' Find last column with data. Already know there is data
RngSrc = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByColumns, xlPrevious)
ColSrcLast = RngSrc.Column
If HeaderCopied Then
' Already have header row(s) in destination worksheet
Set RngSrc = .Range(.Cells(RowDataFirst, 1), .Cells(RowSrcLast, ColSrcLast))
Else
' Do not have header row(s) in destination worksheet. Include them in copy.
Set RngSrc = .Range(.Cells(1, 1), .Cells(RowSrcLast, ColSrcLast))
HeaderCopied = True
End If
RngSrc.Copy Destination:=WshtDest.Cells(RowDestNext, 1) ' Copy data and formats
RowDestNext = RowDestNext + RngSrc.Rows.Count ' Step ready for next copy
End If
End With ' WshtSrc
WbkSrc.Close SaveChanges:=False
Set WshtSrc = Nothing
Set WbkSrc = Nothing
End If ' FileName <> WbkDest.Name
FileName = Dir$
Loop ' While FileName <> "" And FileName <> WbkDest.Name
With WshtDest
.Cells.EntireColumn.AutoFit
End With
Application.ScreenUpdating = True
End Sub
元の回答に対するOPのコメントに対応する新しいセクション
元の回答に含めるべきだったことがいくつかありますが、あなたは私が期待できなかった分野にも迷い込んでいます。また、私自身のテストで見逃したバグも発見しました。
「ワークブック名のようなものに置き換えるべきでしたか...」</p>
これをはっきりさせておくべきでした。ではConst WshtDestName As String = "Data"、「データ」は、データを蓄積するワークシートの私の名前です。ワークシートの「データ」をあなたの名前に置き換えるように言っておくべきでした。
あなたのコメントは、あなたが置き換えたことを示唆しています:
Set WshtDest = WbkDest.Worksheets(WshtDestName)
と
Set WshtDest = WbkDest.Worksheets("Sheet1")
Constその場合は、代わりにステートメントを更新してください。ステートメントを使用する目的はConst、コードの本体から変更される可能性のあるものを分離することです。これにより、メンテナンスが容易になります。
デフォルト名の「Sheet1」、「Sheet2」などは使用しないでください。データとマクロが複雑になるにつれて、ワークシートの名前がワークシートの内容を反映していると、作業がずっと楽になります。
[ OP からのメモ:マスター wksht の名前を「Combined」に、ソースの wkshts の名前を「Node Export By Hub」に変更し、定数の「Sheet1」の名前をそれらの名前に置き換えました。
WbkDest.Nameマスター ワークブックの名前として使用します。これを実際のワークブック名に変更する必要はありません。このようなプロパティを使用すると、ワークブックの名前を変更するとプロパティ値が変更されるため、コードの保守がはるかに簡単になります。
Run Time Error 9 Subscript out of Range エラーを受け取り、それをデバッグすると、Set WshtDest = WbkDest.Worksheets(WshtDestName) が強調表示されました。
この段落は、VBA に関する現在の知識を超えている可能性があります。それを読んで、理解できることを抽出してください。配列とコレクションに進むにつれて、より明確になります。 または、ほとんどのプログラミング言語がリストと呼ぶものですWorksheets。Collectionコレクションは、途中で新しい値を追加して既存の値を削除できる点を除けば、配列に似ています。配列内では、エントリはインデックス番号を介してのみアクセスできます。例: MyArray(5). コレクション内のエントリにはインデックスを介してアクセスできますが、コレクション エントリにはkey. Worksheets(5)ワークシート 5 がないため、私がこれを記述した場合、エラー 9 が発生します。マクロを実行したときWshtDestName、値は「データ」でした。なかっWorksheets("Data")たので、エラー 9 が発生しました。Constステートメントを更新すると、このエラーが発生します。Worksheets("Sheet1")存在します。
最初の定数がSubの後にあるはずだったかどうかわからなかったので、Subの下のDimの後に移動しましたが、何も起こりませんでした。
ここで、「スコープ」のトピックに迷い込んでしまいました。サブルーチンまたは関数内で定数または変数を宣言すると、そのサブルーチンまたは関数内でのみ表示されます。定数または変数の「スコープ」は関数です。モジュールの先頭で定数または変数を宣言すると、そのモジュール内のすべてのサブルーチンまたは関数からは見えますが、他のモジュールでは見えません。定数または変数の「スコープ」はモジュールです。宣言に「Public」を追加すると、定数または変数は、ワークブック内のすべてのモジュールまたはユーザー フォームのすべてのサブルーチンまたは関数に表示されます。定数または変数の「スコープ」はワークブックです。
[ OP からのメモ: Google 経由で Sub の前に定数に関する情報を見つけることができなかったので、これは興味深いことです。ありがとうございました。]
このワークブックには、サブルーチンとモジュールが 1 つだけあり、ユーザー フォームがないため、定数宣言をどこに配置しても問題ありません。定数または変数の最も適切なスコープは複雑な問題であり、紹介を試みるつもりはありません。これらの定数は、ワークブックに関する私の仮定を記録しているとだけ言っておきます。複数のモジュールがあった場合、それらを Public として定義したでしょう。
私の仮定をすべて確認する必要があります。私はあなたのデータを持っていません。あなたのデータと一致すると思われるデータを作成しました。しかし、データに関する私の仮定が間違っていると、マクロは機能しません。
F8 キーを押してコードの各行を確認すると、画面のヒントに次のように表示Application.ScreenUpdating = Falseされます。Application.ScreenUpdating = True
ステートメントが黄色の場合は、まだ実行されていません。F8ほとんどのステートメントでは、もう一度押して実行する前に実際に修正することができます。したがって、黄色があり、 A = A + 2「私が意味した」と思うA = A + 3場合は、実行前にステートメントを修正できます。
Trueのデフォルト値なApplication.ScreenUpdatingので、実行前に表示される値ですApplication.ScreenUpdating = False。
Set WbkDest = ThisWorkbook の場合、画面のヒントには WbkSet = Nothing および ThisWorkbook = が表示されますが、これらのことは黄色で強調表示されている場合にのみ表示されます。F8 を押してその行から移動すると、カーソルをその上に置いても何も表示されません。
通常の変数 (データ型 = Long、String、Boolean など) にカーソルを合わせると、インタープリターはその値を表示します。オブジェクト (WbkDest など) には多くのプロパティがあります。インタプリタはどちらを表示する必要がありますか? などの一部のオブジェクトにRangeは、既定のプロパティがあります。RangeこれはValue、範囲にカーソルを合わせると値が表示されるためです。ワークブックには既定のプロパティがないため、インタープリターは何も表示しません。
イミディエイト ウィンドウに移動し、 を入力? WbkDest.NameしてクリックしますEnter。インタープリターはワークブックの名前を表示します。表示されているブックの任意のプロパティの値を取得できます。WbkDest.Worksheets.Countやなどのサブプロパティを表示することもできますWbkDest.Worksheets("Sheet1").Range("A1").Value。
[ OP からのメモ: 1 番目のエラー- 「? WbkDest.Name' を入力し、イミディエイト ウィンドウで Enter キーを押します。最初Dimで宣言されており、後はSet=This WorkbookでWbkDestを認識していないのでしょうか。名前を MASTER.xlsm から MASTER_DESKTOP TEST.xlsm に変更しましたが、このコードでは明示的に言及していないので問題ありませんよね?]
TD から上記のメモへの応答は Dim X As Type、X 用にいくらかのスペースを確保し、その値をタイプのデフォルトに設定します。Long 型の場合、値は 0 になります。Object 型 (Workbook は Object のサブタイプ) の場合、既定値は Nothing です。プロパティを持たないものはないため、この段階で? WbkDest.Nameはエラーが発生します。ステートメントSet WbkDest = ThisWorkbookが実行されると、WbkDestにアクセスできるようになりましたThisWorkbook。ThisWorkbookがあるNameので? WbkDest.Name、値があります。あなたは正しいです; コードを変更せずにブックの名前を変更できます。
RngDest = .Cells.Find("*", .Range("A1"), xlFormulas, , xlByRows, xlPrevious) を設定します。ここで、RngDest = ヒントには何も表示されず、xlFormulas の場合は -4123、xlByRows の場合は 1 を除いて画面のヒントには何も表示されません。 、xlPrevious の場合は 2 です。
このことから、マクロの以前の実行からのデータの末尾に新しいデータを追加したいと考えています。私の経験では、これは珍しいことですが、念のためこのオプションのコードを含めました。
[ OP からのメモ: 参考までに、はい、マスターにはヘッダーがあり、データはマクロを介して以前にコピーされたデータの下に追加されます。]
TD から上記のメモへの回答私のコードは必要以上に複雑ですが、必要な機能が含まれています。マスター ワークシートが空の場合、ヘッダー行とデータ行は最初のソース ワークシートからコピーされます。データ行のみが他のワークブックからコピーされます。マスター ワークシートが空でない場合、データ行のみがソース ワークシートからコピーされます。
xlFormulas、xlByRows、および xlPrevious は Excel で定義された定数であるため、パラメーターFindは奇妙な数値ではなく意味のある名前になります。
リストされている他のステートメントから、宛先ワークブックは現在空であると推測します。
[ OP からのメモ: 参考までに、はい、マスター/宛先 wkbk には 1 行目にヘッダー行がありますが、それ以外は最初は空です。]
TD から上記のメモへ の応答 私の最後の応答を参照してください。
Do While FileName <> "" And FileName <> WbkDest.Name ここで、FileName と WbkDest.Name の両方が画面ヒントで "MASTER.xlsm" になります。次に、F8 キーを押すと、コードの残りの部分をジャンプして、With WshtDest .Cells.EntireColumn.AutoFit End With などの末尾に移動します。
この時点で、コードのバグに遭遇しました。私のテストでこのバグが発生しなかった理由がわかりません。
質問する必要があります: なぜループが終了したのですか? 次のファイルで繰り返されないのはなぜですか? ほとんどのコードを省略すると、次のようになります。
Do While FileName <> "" And FileName <> WbkDest.Name
‘ Code to process interesting file
FileName = Dir$
Loop ' While FileName <> "" And FileName <> WbkDest.Name
FileName <> "" FileName = "MASTER.xlsm" であるため True ですがFileName <> WbkDest.Name、"MASTER.xlsm" = WbkDest.Name であるため False です。終了条件に達したため、他のファイルをチェックせずにループが終了します。
私は書くべきだった：
Do While FileName <> ""
If FileName <> WbkDest.Name
‘ Code to process interesting file
End If
FileName = Dir$
Loop ' While FileName <> ""
このコードでは、ワークブック「MASTER.xlsm」は必要に応じて無視されますが、ループはさらにワークブックを探し続けます。
変更した構造に一致するようにマクロを修正し、再試行してください。
[ OP からのメモ: 2 番目のエラーコンパイル エラーを受け取りました - 予想: Then または Go To なので、If FileName <> WbkDest.Name の後に Then を追加したので、読み取ります
If FileName <> WbkDest.Name Then
Set WbkSrc=Workbooks.Open (FolderPath & FileName)
'Rest of Code
これは正しいです？]
TD から上記のメモへの回答 はい、そのとおりです。要約を作成しようとするのではなく、テスト済みのコードを含める必要がありました。
[ OP からのメモ: 3 番目のエラー- すべての編集を追加した後、[実行] の下でコンパイル VBA プロジェクトを実行すると、「コンパイル エラー: Do なしのループ」と表示されました。は一番下にあることを指しており、その隣に「Do」があったことはありません。言い換えれば、「Do」がなかったのに、なぜエラーが発生するのかわかりません。]
TD から上記の注への応答コンパイル エラー「Loop without Do」、「Do without Loop」、「If within End If」、「End If without If」などは混乱を招く可能性があります。do ループ、for ループ、If は適切にネストする必要があります。1 つの構造体を完全に完成できなかった場合、コンパイラはおそらく完璧な外側の構造体について不平を言います。私の推測ではEnd If、新しいIf. コンパイラがヒットすると、ネストされた構造体またはその開始Loop点を探します。End If元のコードを、再度テストした修正済みのコードに置き換えました。新しいコードをコピーして、名前を更新できます。ただし、ループを調べて、私のループと一致させる方がよい場合があります。End If