Which Is Better Coding?

[diddi]

so a thumbs up for SmartIndenter? i will give it a go, on your advice. esp. as norie says that it has preferences. all good. well done to all for their input.

just a bit of a survey question: does anyone indent Open/Close 'nests'? i used not to but found it quite useful

 

[Norie]

What sort of Open/Close 'nests' do you mean?

 

[diddi]

when i am doing random file operations, for example. it just reminds me that i have a file open, so dont forget to close it and then wonder why, 2 months later, my latest sub randomly fails with file already open errors.

not meant to be rocket science, but it helps me make a few less blunders.

 

[Norie]

diddi

If you mean File I/O then yes I would indent it.

 

[dk]

Indenting that sort of thing is a little tricky - Smart Indenter doesn't support it as it's not something that you have to do in order to get it to compile (unlike control structures such as If Then, For Next and Select Case). And there's nothing stopping you opening a file in one procedure and closing it in another (although this would probably not be ideal).

One neat way I've found of handling resources such as this is to create a class module that does the work for you e.g. opens the file, reads, writes and closes but you make sure that you put the Close code in the class's Terminate event. I've been using this for database connections and it works a treat. As soon as the object goes out of scope the connection gets closed and the associated object variables get destroyed. It's actually quite easy to implement and you can then re-use in any project quite easily. The benefits are that even though you plan to code correctly and close every file/database connection if you happen to forget or if your code errors and your error handler is not designed properly then the resource will still be closed off correctly.

EDIT: this was not my idea but I read it in Scott Meyers Effective C++ book. The same technique can also be applied to VBA.

HTH
DK

 

[Greg Truby]

...One neat way I've found of handling resources such as this is to create a class module that does the work for you e.g. opens the file, reads, writes and closes but you make sure that you put the Close code in the class's Terminate event.

Hmmm, now I'm gonna have to experiment with this when I get the chance. I'd seen it before in other apps, but had not adopted it as a general practice. Though I probably will going forward.

In particular - I have an Excel add-in that has about 80 users in the client's company and I've had an issue where if a user crashes Excel at a particular spot they have a common file open and locked and then subsequent users cannot open the file and it raises errors. I have no idea if the Terminate code for the class module will run during an Excel crash. I rather suspect it won't. But certainly worth playing with. The hard part is reliably crashing Excel. (I'm not soliciting help on this - just saying why it caught my eye so. But if anyone has a brilliant solution, feel free to post it for the public's edification.)

 

[Jon von der Heyden]

I have adapted this code for a number of projects to check (1) if a workbook is open and (2) who has it open.

Sub TestVBA()
'// Just change the file to test here
Const strFileToOpen As String = "C:\Data.xls"

    If IsFileOpen(strFileToOpen) Then
        MsgBox strFileToOpen & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileToOpen), vbInformation, "File in Use"
    Else
        MsgBox strFileToOpen & " is not open"
    End If
End Sub

Function IsFileOpen(strFullPathFileName As String) As Boolean
'// VBA version to check if File is Open
'// We can use this for ANY FILE not just Excel!
'// Ivan F Moala
'// http://www.xcelfiles.com

Dim hdlFile As Long

    '// Error is generated if you try
    '// opening a File for ReadWrite lock >> MUST BE OPEN!
    On Error GoTo FileIsOpen:
    hdlFile = FreeFile
    Open strFullPathFileName For Random Access Read Write Lock Read Write As hdlFile
    IsFileOpen = False
    Close hdlFile
    Exit Function
FileIsOpen:
    '// Someone has it open!
    IsFileOpen = True
    Close hdlFile
End Function

Function LastUser(strPath As String) As String
'// Code by Helen
'// This routine gets the Username of the File In Use
'// Credit goes to Helen
Dim text As String
Dim strFlag1 As String, strflag2 As String
Dim i As Integer, j As Integer

strFlag1 = Chr(0) & Chr(0)
strflag2 = Chr(32) & Chr(32)

Open strPath For Binary As #1
    text = Space(LOF(1))
    Get 1, , text
Close #1
j = InStr(1, text, strflag2)
i = InStrRev(text, strFlag1, j) + Len(strFlag1)
LastUser = Mid(text, i, j - i)

End Function

 

[Greg Truby]

Hi Jon,

I was excited about the idea of being able to identify who had a file open. But I'm getting dodgy results with that LastUser function.

If the file in question is a workbook opened in another instance of Excel (test #2) LastUser() Overflows on the integers - easily fixed by declaring i and j and longs. But then it returns a bit of readable text and a bunch of non-readable characters.

Sub TestVBA()
    '// Just change the file to test here
    Const strPath           As String = "C:\Users\Greg\Documents\Excel\VB Playground\", _
          strFileToOpen1    As String = "book1.xls", _
          strFileToOpen2    As String = "RibbonTest.xlsm", _
          strFileToOpen3    As String = "dropdown3.txt"
    Dim strFileName As String
    '// (workbook - not opened)
    Let strFileName = strPath & strFileToOpen1
    If IsFileOpen(strFileName) Then
        MsgBox strFileName & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileName), vbInformation, "File in Use"
    Else
        MsgBox strFileName & " is not open"
    End If
    '// (workbook - opened in a different instance of excel)
    Let strFileName = strPath & strFileToOpen2
    If IsFileOpen(strFileName) Then
        MsgBox strFileName & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileName), vbInformation, "File in Use"
    Else
        MsgBox strFileName & " is not open"
    End If
    '// text file opened here
    Open strFileToOpen3 For Output As #2
    Let strFileName = strPath & strFileToOpen3
    If IsFileOpen(strFileName) Then
        MsgBox strFileName & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileName), vbInformation, "File in Use"
    Else
        MsgBox strFileName & " is not open"
    End If
    Close #2
End Sub

For test #3, LastUser() errors out on the Open ___ For Binary since the file is already open. I tried editing it to be:

Open strPath For Binary Access Read As #1

but that still errors out.

 

[Greg Truby]

As an addendum, the following function does appear to work, but I only tested it lightly and only under Win7. Various versions seem to be floating around the internet. I pulled this from ozgrid, but not sure who is the original author

Function GetFileOwner(fileName As String) As String
 
    Dim secUtil As Object
    Dim secDesc As Object
 
    Set secUtil = CreateObject("ADsSecurityUtility")
    Set secDesc = secUtil.GetSecurityDescriptor(strfilename, 1, 1)
    GetFileOwner = secDesc.Owner
 
End Function

EDIT - I always like to try an early bound version when playing around. Here's what that looks like:

Function GetFileOwner(fileName As String) As String
    
    '// to use early bound, set reference to
    '// Active DS Type Library
    
    Dim secUtil As ActiveDs.ADsSecurityUtility, _
        secDesc As ActiveDs.SecurityDescriptor
    
    Set secUtil = New ActiveDs.ADsSecurityUtility
    Set secDesc = secUtil.GetSecurityDescriptor(fileName, 1, 1)
    GetFileOwner = secDesc.Owner
     
End Function

 

[diddi]

File I/O in a multiuser environment has presented problems for me for a while. i have resorted the the not so ideal method of making very short references to files, keeping them open for the least amount of time possible. this works ok for my needs, but if there is a Class level method for file I/O which can help in multiuser context i would be very keen to see more (hint )