Your code is incredibly fragile
directory = ThisWorkbook.Sheets("Macro").Range("D576").Value
rdate = ThisWorkbook.Sheets("Macro").Range("E47").Value
If a single row/column gets added or deleted or moved, those ranges are going to move and your code is going to fail completely.
If you can, those values should be in a dedicated sheet, not buried below 500 lines of other stuff. If not, name your ranges.

So, let's say you take cell D576
and name it Directory_Path
or something similar. Now, rather than
directory = ThisWorkbook.Sheets("Macro").Range("D576").Value
which is incredibly fragile, you can use
directory = ThisWorkbook.Range("Directory_Path").Value
and so long as nobody actually deletes said row/column, that will always point to the right place.
Use With
Rather than re-referencing the same object over and over, you can use a With
statement to hold a reference. Like so:
Application.ActiveWorkbook.SaveAs Filename:=xTextFile, FileFormat:=xlText
Application.ActiveWorkbook.Saved = True
Application.ActiveWorkbook.Close
Becomes
With Application.ActiveWorkbook
.SaveAs Filename:=xTextFile, FileFormat:=xlText
.Saved = True
.Close
End With
much clearer and easier to read.
Handle things in the right order
Specifically your Application.[Settings]
. Anything *meta* like that should go right at the start and right at the end of the method(s) it applies to. This allows you to confirm, at a glance, what the internal state of your method is and check that things are set/reset correctly. You should also disable Application.EnableEvents
and Application.Calculation
for significant extra speed. Like so:
Sub xlsxTotxt()
With Application
.ScreenUpdating = False
.DisplayAlerts = False
.EnableEvents = False
.Calculation = xlCalculationManual
End With
...
Code
...
With Application
.ScreenUpdating = True
.DisplayAlerts = True
.EnableEvents = True
.Calculation = xlCalculationAutomatic
End With
End Sub
Use descriptive naming
Code should be written for other people (including future you) to read. Names should be descriptive, unambiguous and concise. In that order. I highly recommend the excellent, classic article on naming by Joel Spolsky. But in short, things should sound like what they are.
xTextFile
. What on earth is that? Looking at it I have no idea. If I had to guess, I'd say it's some kind of file
object. Oh, it's the filename
you want to save
your text file under? Why not call it newFilename
? Since it's a full filename (including directory path). Maybe newFullfilename
or just fullFilename
might be even more descriptive.
xWs
suffers from the same problem. I can guess it's a worksheet
object but beyond that? no idea. Since you just use it to iterate over the sheets in your workbook, maybe just call it currentSheet
?
fname
. Same thing. filename
. Since it's the filename for the workbook you're opening, how about targetWorkbookFilename
? Sure, it's long, but screen real estate is cheap, cognitive processing is not and that name's an awful lot easier to understand and work with.
Also, move your declarations near to where they're used. This helps you *see* the different parts of your method and helps you keep track of where you are, refer back to your declarations and see ways to split things up into logical sub-methods. Personally, I prefer to keep declarations outside of Loop structures, but that's personal preference.
Objects are your friend
VBA has a huge, extensive, comprehensive object model for every Office Application. Use it.
ThisWorkbook.Sheets("Macro")
You refer to that sheet multiple times. Rather than continually repeating that reference, put it in a worksheet
object and then just refer to the object.
Dim macroSheet As Worksheet
Set macroSheet = ThisWorkbook.Sheets("Macro")
Dim dateString As String
dateString = macroSheet.Range("E47").Value
Same with Workbooks
.
Do While ThisWorkbook.Sheets("Macro").Range("D577").Offset(i).Value <> ""
fname = Sheets("Macro").Range("D577").Offset(i).Value
Workbooks.Open (directory & fname)
For Each xWs In Workbooks(fname).Worksheets
xWs.Copy
xTextFile = directory & rdate & " - " & xWs.name & ".txt"
Application.ActiveWorkbook.SaveAs filename:=xTextFile, FileFormat:=xlText
Application.ActiveWorkbook.Saved = True
Application.ActiveWorkbook.Close
Relying on ActiveWorkbook
being the one you want is very fragile. Just make it a proper object and then your references will always be accurate:
Dim targetWorkbook As Workbook
Set targetWorkbook = Workbooks.Open (directory & fname)
...
For Each currentSheet In targetWorkbook.Sheets()
...
With targetWorkbook
.SaveAs filename:=xTextFile, FileFormat:=xlText
.Saved = True
.Close
End With
Indenting
Indenting is a wonderful way to let you see the structure of your Sub. It'll let you pick out nested-logic (Loop
s, If
s, For
s, With
s etc.) and very quickly get a rough idea for where the important stuff in your sub is. Compare:
Do While ThisWorkbook.Sheets("Macro").Range("D577").Offset(i).Value <> ""
fname = Sheets("Macro").Range("D577").Offset(i).Value
Workbooks.Open (directory & fname)
For Each xWs In Workbooks(fname).Worksheets
xWs.Copy
xTextFile = directory & rdate & " - " & xWs.name & ".txt"
Application.ActiveWorkbook.SaveAs filename:=xTextFile, FileFormat:=xlText
Application.ActiveWorkbook.Saved = True
Application.ActiveWorkbook.Close
Next
Workbooks(fname).Close
i = i + 1
Loop
With
Do While ThisWorkbook.Sheets("Macro").Range("D577").Offset(i).Value <> ""
fname = Sheets("Macro").Range("D577").Offset(i).Value
Workbooks.Open (directory & fname)
For Each xWs In Workbooks(fname).Worksheets
xWs.Copy
xTextFile = directory & rdate & " - " & xWs.Name & ".txt"
With Application.ActiveWorkbook
.SaveAs Filename:=xTextFile, FileFormat:=xlText
.Saved = True
.Close
End With
Next
Workbooks(fname).Close
i = i + 1
Loop
And suddenly you can see your program flow, and where the real work is happening, and where loops start/end, in just a glance. Adding some whitespace also goes a long way to helping readability.
Putting it all together
Sub xlsxTotxt()
With Application
.ScreenUpdating = False
.DisplayAlerts = False
.EnableEvents = False
.Calculation = xlCalculationManual
End With
Dim parentDirectoryCell As Range
Set parentDirectoryCell = ThisWorkbook.Range("Directory_Path")
Dim parentDirectoryPath As String
parentDirectoryPath = parentDirectoryCell.Value
Dim dateString As String
dateString = ThisWorkbook.Sheets("Macro").Range("E47").Value
Dim fullFilename As String
Dim currentSheet As Worksheet
Dim targetWorkbook As Workbook
Dim targetWorkbookFilename As String
Dim rowOffset As Long
rowOffset = 1
targetWorkbookFilename = parentDirectoryCell.Offset(rowOffset).Value
Do While targetWorkbookFilename <> ""
Set targetWorkbook = Workbooks.Open(parentDirectoryPath & targetWorkbookFilename)
For Each currentSheet In targetWorkbook.Sheets()
currentSheet.Copy
fullFilename = parentDirectoryPath & dateString & " - " & currentSheet.Name & ".txt"
With targetWorkbook
.SaveAs Filename:=fullFilename, FileFormat:=xlText
.Saved = True
.Close
End With
Next currentSheet
targetWorkbook.Close
rowOffset = rowOffset + 1
targetWorkbookFilename = parentDirectoryCell.Offset(rowOffset).Value
Loop
With Application
.ScreenUpdating = True
.DisplayAlerts = True
.EnableEvents = True
.Calculation = xlCalculationAutomatic
End With
End Sub