1

I am modifying my current code to be more user friendly. My original code had hard coded file paths. The new code below is passing the file paths from a "control" sub where they are designated by an input box. The issue I am having is that now, once in the private sub routine, the If statements are no longer working. The only difference is that the file path is being passed from another sub instead of being hardcoded into this sub. I'm not sure what I am missing. Any help would be great.

Private Sub copyGLbuildings(NewRecPath As String, GLsrcPath As String)

Dim fname1 As Variant
Dim fname2 As Variant


Dim wb1 As Workbook
Dim Wb0 As Workbook


fname1 = Dir(GLsrcPath & "*Buildings*")
fname2 = Dir(NewRecPath & "*Buildings Rec*")

If fname1 <> "" Then
    Set wb1 = Workbooks.Open(GLsrcPath & fname1)
End If

If fname2 <> "" Then
    Set Wb0 = Workbooks.Open(NewRecPath & fname2)
End If

    Wb0.Sheets(1).Name = "Data"
    Wb0.Sheets.Add.Name = "GL"
    wb1.Worksheets(1).UsedRange.Copy
    Wb0.Worksheets("GL").UsedRange.PasteSpecial Paste:=xlPasteColumnWidths, Operation:=xlNone, SkipBlanks:=False, Transpose:=False
    Wb0.Worksheets("GL").UsedRange.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks:=False, Transpose:=False
    Wb0.Worksheets("GL").UsedRange.PasteSpecial Paste:=xlPasteFormats, Operation:=xlNone, SkipBlanks:=False, Transpose:=False
      

    Wb0.Worksheets(1).Activate
    ActiveWorkbook.Windows(1).DisplayGridlines = False
 
 Call CleanFAGL
 
    wb1.Close
    Wb0.Close savechanges:=True
        

End Sub
deadxcell
  • 11
  • 4
  • 2
    Does `NewRecPath` have a backslash at the end of it? What about `GLsrcPath`? – braX Apr 16 '21 at 20:53
  • See `EnsureSlash` here https://stackoverflow.com/questions/67077087/merge-excel-files-into-a-new-excel-file-based-on-filename/67078563#67078563 – Tim Williams Apr 16 '21 at 20:57
  • Put a breakpoint in your code before 'if fname1' and look at the values of fname1 and fname2 in the locals window. Are the values a correctly formed filepath. – freeflow Apr 16 '21 at 20:58
  • @braX - No they do not. I figured when someone copies from file explorer, the end slash is not included; at least it hasn't been when I've tested it. – deadxcell Apr 17 '21 at 13:22
  • @freeflow - Being new to coding, I had no idea you could step into the code like that. Once I did that, it was clear there was no value for fname1 and fname 2. I was missing a back slash in designating the variants. It looks to be working great now! – deadxcell Apr 17 '21 at 13:36

2 Answers2

0

The code looks fine to me. I would advise you put a break point right on the first If Statement and use View > Locals to check what the values of NewRecPath and GLsrcPath are being passed into the Sub Routine as.

Mark
  • 178
  • 1
  • 2
  • 16
0

Copy to Another Workbook

  • Using the Destination (written to) before the Source (read from) as the argument is quite unusual.
  • fname is better written like fName.
  • Neither of the fNames are ever going to be "", since you have previously added the patterns.
  • wb0 is a horrible idea, at least start numbering with 1 in the spirit of Excel and VBA. Note that my choice of variable names is also not quite good, but only you can improve it since you fully understand what it's all about.
  • If it's a worksheet then call it a worksheet (readability) or at least be consistent.
  • After adding a worksheet without arguments, it will 'land' before the selected (active) sheet, so if you only have one sheet in the workbook, it will be the first. If it isn't, why would you gamble with it, if you can settle this explicitly?
  • Have a think about the order of the PasteSpecial lines e.g. you adjusted the column widths before the data was copied.
  • Using sheet or worksheet indexes is prone to errors and confusion which in this case leads me to not knowing which worksheet should be 'deprived' of displaying grid lines.
  • The rest is covered in your comments.
  • Don't worry, we all were at this point at one time.
  • Carefully read the comments and modify the code appropriately.
Option Explicit

' Note that the arguments are switched due to the 'From To' logic.
Private Sub copyGLbuildings( _
        ByVal SourcePath As String, _
        ByVal DestinationPath As String)
    
    ' Validate
    
    ' Destination
    Dim dwbName As String: dwbName = Dir(DestinationPath & "*Buildings Rec*")
    If dwbName = "" Then Exit Sub
    
    ' Source
    Dim swbName As String: swbName = Dir(SourcePath & "*Buildings*")
    If swbName = "" Then Exit Sub
    
    'Application.ScreenUpdating = False
    
    ' Destination
    
    ' Workbook
    If Right(dwbName, 1) <> "\" Then
        dwbName = dwbName & "\"
    End If
    Dim dwb As Workbook: Set dwb = Workbooks.Open(DestinationPath & dwbName)
    
    ' Worksheets
    Dim dws1 As Worksheet: Set dws1 = dwb.Worksheets(1)
    dws1.Name = "Data"
    Dim dws2 As Worksheet
    ' Do it explicitly, even if there is previously only one worksheet.
    Set dws2 = dwb.Worksheets.Add(After:=dws1) ' ??? maybe Before:=dws1
    dws2.Name = "GL"
    
    ' Source
    
    If Right(swbName, 1) <> "\" Then
        swbName = swbName & "\"
    End If
    Dim swb As Workbook: Set swb = Workbooks.Open(SourcePath & swbName)
    ' Copy
    swb.Worksheets(1).UsedRange.Copy
    
    ' Destination
    
    ' Paste
    With dws2.UsedRange
        .PasteSpecial Paste:=xlPasteFormats
        .PasteSpecial Paste:=xlPasteValues
        .PasteSpecial Paste:=xlPasteColumnWidths
    End With
    Application.CutCopyMode = False
    ' Dispay Gridlines
    dws1.Activate ' ??? if "Data" or dws2.Activate ' if "GL": depending on After
    dwb1.Windows(1).DisplayGridlines = False
     
    CleanFAGL ' ??? Not knowing what this does, doesn't help.
     
    Application.DisplayAlerts = False
    swb.Close SaveChanges:=False
    dwb.Close SaveChanges:=True
    Application.DisplayAlerts = True

    'Application.ScreenUpdating = True

End Sub
VBasic2008
  • 44,888
  • 5
  • 17
  • 28
  • 1
    I really appreciate you noting specifics and laying out, for me, how a carefully organized code set should look. Being so new to coding, I've basically been getting my direction from all over the place and my final products tend to be a Frankenstein of various codes with adjustments from my own readings on blogs and walkthroughs. So, again, thank you. – deadxcell Apr 17 '21 at 14:06