0

I am trying to copy specific cells from a file from the day before and paste them on the same spot in my current workbook as text.

I only want this to be done on Thursday.

Sub OpenFile()

If Weekday(Now()) = vbThursday Then

    FileYear = Year(Date)
    FileDate = Format(Date, "yymmdd")
    FilePath = "I:\Example\2020\" & Format(Now() - 1, "yymmdd") & " " & _
      "Sequentieanalyse werkblad.xlsm"

    Workbooks.Open (FilePath)

    Range("P48:Z57").Select
    Selection.Copy
    ActiveWorkbook.Close False

    Sheets("Monsterlijst").Select
    Range("P48:Z57").Select
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
       :=False, Transpose:=False

End If

End Sub

When I run my code I get a messagebox that there is a lot of information on the clipboard and I have to select yes, no or abort, followed by a runtime error 1004 with either of the options.
When I use Application.DisplayAlerts = False I also get a Runtime error 1004.

There are no error messages when I remove ActiveWorkbook.Close False, but my info will paste back in the workbook I am copying from instead of in my current workbook.
I also want the file from the previous day closed to prevent confusion.

The copy part does works, because if I cancel the error message I can paste it manually.

How would I adapt the code to run on Friday instead of Thursday if Thursday is a bank holiday?

Community
  • 1
  • 1
Caroline
  • 1
  • 1
  • 1
    Please see https://stackoverflow.com/q/10714251/11683. – GSerg Jan 16 '20 at 12:43
  • 1
    Copy, then paste, then close. Do not `Select` while doing any of that. Save the result of `Workbooks.Open (FilePath)` into a variable (`As Workbook`) to use it as the source workbook. Use `ThisWorkbook` as the target workbook. – GSerg Jan 16 '20 at 12:46
  • As for the second question, this is an entirely different issue which should be posted on its own question. – Plutian Jan 16 '20 at 12:54

1 Answers1

0

Your code is extremely inefficient. Please take @GSerg comment to heart and read up on how to avoid using select.

The issue you are facing in particular here is that you close the workbook before you paste the copied cells anywhere, resulting in your error. This can be prevented entirely by moving the Paste action above the Close action:

Option Explicit

Sub OpenFile()
Dim ref As Workbook
Dim filepath As String

If Weekday(Now()) = vbThursday Then

filepath = "I:\Example\2020\" & Format(Now() - 1, "yymmdd") & " " & "Sequentieanalyse werkblad.xlsm"

Set ref = Workbooks.Open(filepath)

Range("P48:Z57").Copy
ThisWorkbook.Sheets("Monsterlijst").Range("P48:Z57").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks:=False, Transpose:=False

ref.Close False
End If

End Sub

(code also optimised to avoid using select)

Plutian
  • 2,276
  • 3
  • 14
  • 23
  • @BigBen Proofreading saves lives, I should do it more often :P – Plutian Jan 16 '20 at 13:01
  • As an alternative you could store the data in an array first and then write the array to the desired output range. Avoids several potential issues and makes debugging life easier. – Christov Jan 16 '20 at 13:05
  • 1
    Thank you all for your quick reply. I will definately check my other codes to see if I used "select" more often and can replace it there too. It still gave me a messagebox about the clipboard, but I added a “Application.CutCopyMode = False" and now everything works perfect! – Caroline Jan 16 '20 at 13:11
  • 1
    `Range("P48:Z57").Copy` still relies on implicit active sheet (the fact that `Workbooks.Open` makes the opened workbook active). Better to `ref.Worksheets("name").Range("P48:Z57").Copy`. – GSerg Jan 16 '20 at 13:35
  • @GSerg agreed, also if the first sheet is needed `ref.Sheet1` would work. But without knowing which sheet is needed I can't put that in the answer. – Plutian Jan 16 '20 at 13:38