1

This code worked once and then stopped. It runs with no action or errors.

I would like if column "a" of the "export" sheet has a yes to copy the cells from B to J to the next clear line in workbook MOSTEST sheet1 (named 11.2022).

Sub DateSave()

Dim LastRow As Integer, i As Integer, erow As Integer
LastRow = Worksheets("EXPORT").Range("A" & Rows.Count).End(xlUp).Row

For i = 1 To LastRow

    If Cells(i, 1).Value = "YES" Then
        Range(Cells(i, 2), Cells(i, 10)).Select
        Selection.Copy

        Workbooks.Open Filename:="F:\Orders\MOSTEST.xlsx"
        Worksheets("11.2022").Select
        erow = ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Offset(1, 0).Row
        ActiveSheet.Cells(erow, 1).Select
        Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
        ActiveWorkbook.Save
        ActiveWorkbook.Close
        Application.CutCopyMode = False
    End If

Next i

End Sub

If changed the "Worksheets("11.2022").Select" to sheet1 which I would prefer as I wouldn't have to change it every month.

Community
  • 1
  • 1
TomBudge
  • 45
  • 5

1 Answers1

1

You should try to avoid using select, see other post

I adjusted your code where needed, I'm still trying to figure out best practice (i.e. it would be better adding the cell ranges to a range variable and then pasting them in one go but I'm not quite there yet) when it comes to minimizing code so if others can do better, feel free :)

Sub DateSave()

    Dim LastRow As Long, i As Long, erow As Long
    Dim wsStr As String
    Dim ws As Worksheet, wsC As Worksheet
    Dim wb As Workbook, wbM As Workbook
    LastRow = Worksheets("EXPORT").Range("A" & Rows.Count).End(xlUp).Row
    
    Set wb = ActiveWorkbook
    Set wsC = wb.Sheets("EXPORT")
    Workbooks.Open Filename:="F:\Orders\MOSTEST.xlsx" 'Don't keep opening and saving/closing your workbook per copy, that would heavily increase runtime
    Set wbM = Workbooks("MOSTEST.xlsx")
    wsStr = Month(Date) & "." & Year(Date)
    Set ws = wbM.Worksheets(wsStr) 'If your currentmonth will always be the first sheet then you can use wbM.Sheets(1)
    erow = ws.Cells(Rows.Count, 1).End(xlUp).Row
    wb.Activate

    For i = 1 To LastRow
        If wsC.Cells(i, 1).Value = "YES" Then
            erow = erow + 1
            wsC.Range(wsC.Cells(i, 2), wsC.Cells(i, 10)).Copy 'avoid select
            ws.Range("A" & erow).PasteSpecial xlPasteValues, Operation:=xlNone, SkipBlanks _
                    :=False, Transpose:=False
        End If
    Next i
    
    wbM.Save
    wbM.Close
    Application.CutCopyMode = False
End Sub

If you have questions, feel free to ask!

Notus_Panda
  • 1,402
  • 1
  • 3
  • 12
  • Hi, Thanks very much. it runs very fast but doesnt seem to be either coping or pasting or both? it opens the MOSTEST and closes it but nothing changes. – TomBudge Nov 17 '22 at 09:48
  • I made a small edit, forgot to Activate the workbook you're copying from, my bad. Does it work now? – Notus_Panda Nov 17 '22 at 10:27
  • For some reason no......opens and closes but doesnt change the MOSTEST file. – TomBudge Nov 17 '22 at 16:49
  • move both files to the same place as the master is on onedrive and MOSTEST is on a nas, same thing happens so ruled that out. – TomBudge Nov 17 '22 at 16:55
  • Is the worksheet protected or is the workbook already opened somewhere else? If not, try going through the code with F8 and see what it actually does do, it worked fine when I was testing it, I'll test again tomorrow. – Notus_Panda Nov 17 '22 at 17:10
  • Added the connection with the worksheet EXPORT as wsC, now you can run the code even if you're not on the EXPORT sheet. However, the code works for me perfectly fine, even had the MOSTEST saved on a different sheet and it copied the Cells B:J just fine on corresponding rows with "YES" in A. Check if your A column does contain the "YES"; if sheets would be protected, you'd get some notification normally so it shouldn't be that. I also got an error when I forget that the sheet 11.2022 should exist so can't be that either. – Notus_Panda Nov 18 '22 at 08:17
  • I cant thanks you enough, it works great now and i undrstand everything you have done. Thanks so much for all your help. – TomBudge Nov 18 '22 at 11:15
  • You're welcome, helping others is how I keep learning as well when I'm not otherwise occupied :) Don't forget that your path is hardcoded so if anything changes to MOSTEST (name/file location), this will stop working again. – Notus_Panda Nov 18 '22 at 11:30
  • Yes me too, that was a test file so i have ponited it at the correct file now. on to the next callenge. thanks again. – TomBudge Nov 18 '22 at 12:16
  • incase you would like to new callenge :) https://stackoverflow.com/questions/74492464/looping-and-pasting-based-on-cell-value – TomBudge Nov 18 '22 at 16:15