6
\$\begingroup\$

After implementing some logging in my system, I got tired of digging through very long files trying to determine where specific runs started/stopped to look for differences, so I began adding a date/time stamp to the file name so I'd get a new log file each time the processing kicked off. This was wonderful, but solving one problem created a new one - now I have a proliferation of log files and I have to keep the collection of files under control.

I've written the following procedure to do the pruning and it works quite well. I'm wondering if there are any gotcha's I may have missed in my extensive (2 hours of) testing, or if there are any more efficient ways of doing this.

I believe the implementation is fairly efficient. There's no need to keep a list of the naxLogsToKeep newest (or oldest) files in memory, only the one I'm looking at now and the one known to be the oldest I've looked at so far. By not having an array, collection, dictionary or other structure of all the files, there's no need to sort anything, so this should be fairly efficient. But, I don't know it all, so I'm open to suggestions.

There are some situations that I'm aware of:

  • What happens if filePath doesn't have a trailing slash? It's already been checked in the calling procedure. I suppose it should be checked here again, just in case I ever reuse this code.
  • The error handling is... weak. Yup. I threw something in there just for giggles and grins, but will probably remove it and let it bubble up to be dealt with at a higher level.
  • It doesn't work well if there are more than maxLogsToKeep logs. Since Dir() gives you no indication whatsoever of what order you'll get files, if there are extra files than this will delete the oldest one(s) it's come across when it hits maxLogsToKeep. I'm pretty sure, in my limited testing, that it will work just fine, once there are only maxLogsToKeep log files in the given directory.

With that, the code:

Private Sub DeleteExtraLogFiles(ByVal filePath As String, ByVal fileNamePattern As String, ByVal maxLogsToKeep As Long)

  On Error GoTo ErrorHandler

  Dim fileCount As Long

  Dim oldestFileDate As Date
  oldestFileDate = #12/31/3999#

  Dim currentFile As String
  currentFile = Dir(filePath & fileNamePattern)

  Do While Len(currentFile) > 0
    fileCount = fileCount + 1
    Dim currentFileDate As Date
    currentFileDate = FileDateTime(filePath & currentFile)
    If fileCount < maxLogsToKeep Then
      If currentFileDate < oldestFileDate Then
        Dim oldestFile As String
        oldestFile = currentFile
        oldestFileDate = currentFileDate
      End If
    Else
      If currentFileDate < oldestFileDate Then
        Kill filePath & currentFile
      Else
        Kill filePath & oldestFile
        oldestFile = currentFile
        oldestFileDate = currentFileDate
      End If
    End If
    currentFile = Dir
  Loop

CleanExit:   
  Exit Sub

ErrorHandler:
  With Err
    MsgBox "Error: " & .Number & vbCrLf & .Description
  End With
  Resume CleanExit

End Sub
\$\endgroup\$
  • \$\begingroup\$ I'm just curious - what are these logs of? Actions in Excel? Something else in Office? Something else not in Office? \$\endgroup\$ – Raystafarian Apr 24 '18 at 21:40
  • \$\begingroup\$ @Raystafarian - The logging routine is used in both Access & Excel for various processes I have that collect data from sources, generate reports from that data, etc. Here be indications of who's done what, when, and what may have gone wrong. 99.9% of the time, who = me, so there's no need for long term storage, otherwise the log info would be pushed into a table. ;) \$\endgroup\$ – FreeMan Apr 25 '18 at 12:02
  • 1
    \$\begingroup\$ Another approach would be creating a folder structure which would indicate the Year and Month where files would be saved by your program. E.g. 2018, inside of the 2018 folder there would be a folder for each month. When the file writes, it determines which folder to write to. That way you can selectively read/delete entire folders, instead of being concerned of reading each file in a large directory. Kinda like a partition. \$\endgroup\$ – Ryan Wildry Apr 26 '18 at 12:38
  • \$\begingroup\$ Interesting idea, @RyanWildry. However, for the current situation, at least, I really don't need to keep them long term. That does sound like a good idea and I'll keep it in mind as an option if I need to do audit type logs that need to be kept long term. \$\endgroup\$ – FreeMan Apr 26 '18 at 12:43
1
\$\begingroup\$

Let's create a scenario. You have lots of log files to prune - some from January, February and March. You have, for whatever reason, way more than maxLogsToKeep. I can see a possibility that your routine would delete the February files and keep the January ones because you do not sort and you trust the order in which Dir returns the files. However, this may not be important to you.

Firstly, reference Microsoft Scripting Runtime in your VBA project. This gives you access to the FileSystemObject and related classes. This gives you more helper functions that will make the code easier to read and maintain (e.g. the File class allows you to access file information without additional reference to the file path; and a File.Delete).

The only way around that scenario I noted is to sort. While the FSO has a Files collection, I don't see any additional functionality that allows you to sort the collection, so you can create your own collection (array in VBA for sorting) and sort that.

Some web references (links are now old - while the fundamentals stay the same the coding could take advantage of newer functionality in VBA):

If I was writing this, then I would traverse the Files collection and build the Array (of File) as I went (the Files.Count will let you dimension the array properly in the first place) but perhaps a quick sort would be more efficient. After you have built the sorted array, you can traverse the array from the end until maxLogsToKeep and delete the files (File.Delete) (*).

Yes, a little more complicated, but addresses a scenario where you might retain ancient logs at the expense of more recent ones.

(*) I know I am working with an array rather than a For Each or other collection enumeration, but I now have a habit of working from the end when deleting things.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your feedback. As my 3rd bullet mentions, this is something that I'd considered, but I'm not particularly concerned about. These logs aren't critical (they don't provide legal evidence or audit trails), so for my purposes, if I don't delete the absolute oldest set in the first run, I'm not going to panic. It also greatly simplifies the process by not having to keep an array or collection and not having to do any sorting. The quicksort link is very handy, though! \$\endgroup\$ – FreeMan Apr 25 '18 at 12:30
  • \$\begingroup\$ @FreeMan. I recommend looking at FSO anyway. Even if you don't sort it may have a couple of additional utilities that make the current code cleaner. \$\endgroup\$ – AJD Apr 25 '18 at 19:49
  • \$\begingroup\$ If you've got some suggestions on cleaning it up with FSO, I'm all ears. I did look into it, and Dir seemed to be the simplest method that would get the job done. I may well have missed something obvious... \$\endgroup\$ – FreeMan Apr 26 '18 at 12:41
  • \$\begingroup\$ @FreeMan: In this relatively simple example, the use of the Files collection in the folder would make the loop cleaner. You could loop backwards (For f = Files.Count to 1 Step -1) through each File object which not only includes the details you check (dates) but includes a f.Delete method, meaning you don't require a Kill routine. \$\endgroup\$ – AJD Apr 26 '18 at 19:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.