Code Review - Files from Directories

[Philcjr]

All,

I have worked on this code for most of the afternoon and I was wondering if anyone could look at this and recommend ANY ways of making this code better/faster. Unfortunatly, the files I am extracting are on a shared server which could produce a slower responce time... with that said, I would like to have the best code possible. There are NO errors or problems... this code works. Any suggestions, are greatly appreciated.

[VBA]
'-------------------------------------------------------------
Function GetFileName(Strpath As String)

Dim iStart As Integer
Dim Strchar As String

For iStart = Len(Strpath) To 1 Step -1
Strchar = Mid(Strpath, iStart, 1)
If Strchar = "\" Then Exit For
Next iStart

GetFileName = Mid(Strpath, iStart + 1, Len(Strpath) - iStart)
End Function
'-------------------------------------------------------------
Sub Button1_Click()

Dim vName As Variant
Dim pFile As Integer, aFile As Integer, x As Integer, C As Integer
Dim pDirectory As String, aDirectory As String

'Set path for files (Preliminary and Authorized)
pDirectory = "S:\Phil\Forms - Issued\LDF\" & vName(x) & "\Preliminary"
aDirectory = "S:\Phil\Forms - Issued\LDF\" & vName(x) & "\Authorized"

Sheets("Data Sheet").Select
Range("A:L").Clear
Range("A2").Select

vName = Array("Dave Dittelman", "Dick Towne", "Ed Candelaria", "Rick Nehls", _
"Ryan Tesiero", "Display Services")
For x = LBound(vName) To UBound(vName)

'Lists all the files in a Preliminary state
ActiveCell.Offset(0, C).Value = vName(x) & " - Preliminary"
With Application.FileSearch
.LookIn = pDirectory
.Execute
For pFile = 1 To .FoundFiles.Count
ActiveCell.Offset((pFile + 1), C).Value = GetFileName(.FoundFiles(pFile))
Next pFile
End With

'Move to next column, and list the files in an Autorized state
ActiveCell.Offset(0, C + 1).Value = vName(x) & " - Authorized"
With Application.FileSearch
.LookIn = aDirectory
.Execute
For aFile = 1 To .FoundFiles.Count
ActiveCell.Offset((aFile + 1), C + 1).Value = GetFileName(.FoundFiles(aFile))
Next aFile
End With

Let C = C + 1 'Increase C to goto next column
Next x

Range("A2:L2").Select
With Selection
.HorizontalAlignment = xlCenter
.VerticalAlignment = xlCenter
.WrapText = False
.Interior.ColorIndex = 1
.Interior.Pattern = xlSolid
.Font.Name = "arial"
.Font.Size = 10
.Font.Bold = True
.Font.ColorIndex = 2
End With

Columns("A:N").EntireColumn.AutoFit
Range("A1").Select
End Sub

[/VBA]

Thanks, Phil

[Tommy]

Hi Phil,
I haven't looked at the Button1_Click, but I did look at GetFileName. I have seen this method several times on the net so I am not the original author nor do I know who is.
[VBA]
Function GetFileName(Strpath As String) As String
GetFileName = Mid(Strpath, InStrRev(Strpath, "\") + 1)
End Function
[/VBA]

When I get the chance I will look at the other one this one I knew off the top of my head.

[mdmackillop]

Hi Phil,
Some minor tweaks here. One other suggestion. How about reading all of the file names into arrays and returning the data from these arrays. This might reduce your network traffic to speed things up.

[VBA]
'-------------------------------------------------------------
Function GetFileName(Strpath As String)

Dim MyFile

MyFile = Split(Strpath, "\")
GetFileName = MyFile(UBound(MyFile))
End Function
'-------------------------------------------------------------
Sub Button1_Click()

Dim vName As Variant
Dim pFile As Long, aFile As Long, x As Long, C As Long
Dim pDirectory As String, aDirectory As String

'Set path for files (Preliminary and Authorized)
'Lines moved as x has no value here

Sheets("Data Sheet").Select
Range("A:L").Clear
Range("A2").Select

vName = Array("Dave Dittelman", "Dick Towne", "Ed Candelaria", "Rick Nehls", _
"Ryan Tesiero", "Display Services")
For x = LBound(vName) To UBound(vName)

pDirectory = "S:\Phil\Forms - Issued\LDF\" & vName(x) & "\Preliminary"
'Lists all the files in a Preliminary state
ActiveCell.Offset(0, C).Value = vName(x) & " - Preliminary"
With Application.FileSearch
.LookIn = pDirectory
.Execute
For pFile = 1 To .FoundFiles.Count
ActiveCell.Offset((pFile + 1), C).Value = GetFileName(.FoundFiles(pFile))
Next pFile
End With

aDirectory = "S:\Phil\Forms - Issued\LDF\" & vName(x) & "\Authorized"
'Move to next column, and list the files in an Autorized state
ActiveCell.Offset(0, C + 1).Value = vName(x) & " - Authorized"
With Application.FileSearch
.LookIn = aDirectory
.Execute
For aFile = 1 To .FoundFiles.Count
ActiveCell.Offset((aFile + 1), C + 1).Value = GetFileName(.FoundFiles(aFile))
Next aFile
End With

Let C = C + 1 'Increase C to goto next column
Next x


With Range("A2:L2")
.HorizontalAlignment = xlCenter
.VerticalAlignment = xlCenter
.WrapText = False
.Interior.ColorIndex = 1
.Interior.Pattern = xlSolid
.Font.Name = "arial"
.Font.Size = 10
.Font.Bold = True
.Font.ColorIndex = 2
End With

Columns("A:N").EntireColumn.AutoFit
Range("A1").Select
End Sub
[/VBA]

[Philcjr]

Tommy,

Thanks for the tweak on the Function... I will test at work tomorrow.

Phil

[Bob Phillips]

You could also look at the usaul suspects, screenupdating and calculation, and also drop the items into an array in the loop, not the cells, and then dump the arry wholesale onto the worksheet. Time both and see if there is any significant improvement.

[Philcjr]

MD,

Some minor tweaks here. THANKS!

One other suggestion. How about reading all of the file names into arrays and returning the data from these arrays. This might reduce your network traffic to speed things up.
UMMMMM... I AM NOT SURE HOW TO DO THIS, ANY HINTS OR SAMPLE I WILL TRY AND FIGURE IT OUT THE REST.

Phil

[Philcjr]

XLD,

ahhh yes. I will turn off screen updating (Good Call )

"...and also drop the items into an array in the loop, not the cells, and then dump the arry wholesale onto the worksheet." How can I do this? Just looking for some direction.

Thanks,
Phil

Phil

[Bob Phillips]

Not exactly your code, but similar and should get yopu going.

[VBA]
Dim aryData
Dim iEnd As Long
Dim i As Long

ReDim aryData(1 To 2, 1 To 1)

iEnd = 20
ReDim Preserve aryData(1 To 2, 1 To UBound(aryData, 2) + iEnd)
For i = 1 To iEnd
aryData(1, i + 1) = "Loop 1 - " & i
Next i


iEnd = 25
If iEnd + 1 > UBound(aryData, 2) Then
ReDim Preserve aryData(1 To 2, 1 To iEnd + 1)
End If
For i = 1 To iEnd
aryData(2, i + 1) = "Loop 2 - " & i
Next i

Range("A1").Resize(UBound(aryData, 2), 2) = Application.Transpose(aryData)
[/VBA]

[Philcjr]

XLD,

THANKS! I will be breaking out the books and reading up on arrays.

IF, and that is a BIG IF, you have time, any other clues would be a help... for instance, why/what is "ReDim", I have yet to use something like this. "Preserve" what does this do?

Again,
THANK YOU! Phil

[mdmackillop]

Hi Phil,
Redim redimensions an array to accommodate the data, either increase or decrease, if the final size is not known at the outset.
Preserve retains the data. Without this, all entries are cleared when the array is redimmed.

[TonyJollans]

How about reading all of the file names into arrays and returning the data from these arrays. This might reduce your network traffic to speed things up.

Doesn't the FileSearch effectively do that anyway - into the FoundFiles array?

[Bob Phillips]

Doesn't the FileSearch effectively do that anyway - into the FoundFiles array?

Effectively maybe, but FoundFiles is not an array.

Philcjr,

If I get time tomorrow morning, I will re-cut your code and then you can time it. Bedtime now.

[Philcjr]

Thanks XLD,

I look foward to your code... I can always use a lesson in Arrays.

Thank you,
Phil

[Zack Barresse]

Btw, to but in a little bit here, some of the suggestions won't work with xl 97, i.e. the Split and InStrRev functions. Just FYI.

[Bob Phillips]

Thanks XLD,

I look foward to your code... I can always use a lesson in Arrays.

Thank you,
Phil

Phil,

A question.

I have cut some code and noticed some odd things going on. So I ran your code and it does the same thing.

It loads all the Preliminary files into column A for Dave Dittelman, then the Authorised files for same. Then it loads the same files into column B for Dick Towne, overwriting DD's Authorised. The last guy's Authorised remains (nothing to overwrite it).

So two questions, is the Preliminary meant to overwrite AUthorised?
Should the setting of pDirectory be after vName is setup so that a different directory is searched each time?

Thanks

Bob

[Philcjr]

Bob,

Answer to question 1.
The intent of this macro was to be able to compare the Preliminary Files to the Authorized Files. With that said, Dave should have a column for Preliminary and a column for Authorized... and the same for the other people.

Answer to question 2.
"Should the setting of pDirectory be after vName is setup so that a different directory is searched each time"? Yes, good catch... I think mdmackillop caught that also.

Thanks for all you help, if you want I can post the file when I am at work tomorrow.

Phil

[Bob Phillips]

Bob,

Answer to question 1.
The intent of this macro was to be able to compare the Preliminary Files to the Authorized Files. With that said, Dave should have a column for Preliminary and a column for Authorized... and the same for the other people.

Answer to question 2.
"Should the setting of pDirectory be after vName is setup so that a different directory is searched each time"? Yes, good catch... I think mdmackillop caught that also.

Thanks for all you help, if you want I can post the file when I am at work tomorrow.

Phil

No that's okay, I have test data now. Not as much as you, but enouigh to prove it works.

As usual you catch me at bedtime, so I will have to continue tomorrow.

Cheers

Bob

[Philcjr]

Bob,

Any luck with this?

Phil