Optimizing code

[withanh]

I'm working on optimizing some code I wrote and I have a question about storing a value in a variable vs looking it up multiple times. Just thinking about it, it seems that looking it up once and storing it in a variable is more efficient, but I guess I don't really know.

The code below is how I started, the 2nd code block is what I revised the sub to. The first has one less line of code, but the second seems like it would be more efficient since it's only doing one lookup.

I'd like opinions on which is better and why.

Thanks!

Sub ProcessFiles()
  Set objFolder = objFSO.GetFolder("D:\Downloads")
  For Each objFile In objFolder.Files
    'Launch SQL process to import data in file
    'SQL launch code goes here
    strNewName = fldProcessed & "\" & Left(objFile.Name,Len(objFile.Name)-4) & ".old"
    If FileExists(strNewName) Then
      DeleteFile(strNewName)
    End If
    objFile.Move strNewName
  Next
End Sub

Sub ProcessFiles()
  Set objFolder = objFSO.GetFolder("D:\Downloads")
  For Each objFile In objFolder.Files
    strFileName = objFile.Name
    'Launch SQL process to import data in file
    'SQL launch code goes here
    strNewName = fldProcessed & "\" & Left(strFileName,Len(strFileName)-4) & ".old"
    If FileExists(strNewName) Then
      DeleteFile(strNewName)
    End If
    objFile.Move strNewName
  Next
End Sub

 

[mrmovie]

with regards to which is most efficient then testing it would be the only true way of determining.

i dont relate efficiency to the number of lines of code. well not in regards to script execution.

no one could accuse you of being inefficient in this instance due to the extra line of code in the second example.

i dont think you will see much impact on speed with your suggested change (in this example), i tend to rely on the .Files collection and .Name property of these objects to be pretty efficient. However, your second approach would be best practice i would say in the long run. A good example of collections and enumerations costing efficiency compared with reading once and storing in your own variables is the groupmembership collection of the old WinNT objects / classes.

i presume you will only need to enumerate the contents of folder D:\Dowwnloads once? if you needed to enumerate the contents more than once then your use of a Sub is not efficient, you would want to read the contents of the sub into your own array and it would be much faster if you needed to check the contents again (a mute point as the contents wouldnt have changed if you had read it into memory)

I presume you will only need to enumerate the contents of the folder D:\Downloads once? If so, then the use of a Sub (As you have presented it is) is a waste. If you arent going to run the code section more than once, then the fact you have statically declared D:\downloads in the sub, and the .old for that matter, and the 4, then you wont be able to 'resuse' this code with different parameters in future scripts / scenarios. I would advise that the three parameters in your sub be defined as parameters in the sub declaration. then when you call the sub you can pass through the data, logically you would then author the script to accept command line parameters (or read data from an external source), this would make your 'work' more reuseable.

 

[withanh]

Thank you, I appreciate the advice.