7
\$\begingroup\$

The Dir() function in VB and VBA can accept wildcard characters (* and ?) but only in the basename of the path.

Dir("path\*.txt")

is a valid request but.

Dir("path\*\*.txt")

will return Error 52 "Bad File Name or Number", because * is an invalid folder or file name. I created the ability to use wildcards in the folder names using my os module and my List class.

Example Call

It's not a true glob search as you can't search like "*.{cpp,hpp}", but this is far better than nothing. Example:

Debug.Print Glob("C:\users\ptwales", "????-herp\*\*.txt").ToString
' ["C:\users\ptwales\derp-herp\flerp\merp.txt", ...]

Client code must specify a starting directory. Current working directory is not implied.

Implementation

Note that os.SubItems and os.SubFolders each now return a List.

Public Function Glob(ByVal root As String, ByVal pattern As String) As List

    Dim pat_list As New List
    pat_list.Extend Split(pattern, os.SEP)

    Set Glob = GlobRecurse(root, pat_list, 1)

End Function
Private Function GlobRecurse(ByVal root As String, ByVal pat_list As List, ByVal index As Integer) As List

    If index = pat_list.Count Then
        Set GlobRecurse = os.SubItems(root, pat_list(index))
    Else

        Set GlobRecurse = New List

        Dim folder As Variant
        For Each folder In os.SubFolders(root, pat_list(index))
            GlobRecurse.Extend GlobRecurse(folder, pat_list, index + 1)
        Next folder

    End If

End Function

It's a bit awkward, particularly that GlobRecurse checks the pat_list.Count each iteration. It feels too much like scheme and I should be using a for loop instead.

\$\endgroup\$
3
  • \$\begingroup\$ Something's not quite right. I don't like that you have a recursive and nonrecursive version of Glob... Yet I do because it hides some complexity that doesn't need to be seen publicly. Hmmm... I'll be back. Need to think on this one. \$\endgroup\$
    – RubberDuck
    Sep 26, 2014 at 23:28
  • 1
    \$\begingroup\$ Can I assume an example call looks like this, Glob("C:\path\to\file\*\*.txt") ? \$\endgroup\$
    – RubberDuck
    Sep 26, 2014 at 23:30
  • 2
    \$\begingroup\$ I figured out why it looks weird to me. In any sensible language, the private sub would actually be an private overloaded version. It's fine. It's just vba being stupid again. (I mean, you could use an optional argument with a default value of 1, but I don't recommend it.) \$\endgroup\$
    – RubberDuck
    Sep 28, 2014 at 0:50

1 Answer 1

4
\$\begingroup\$

Naming, naming, naming.

I've never heard of a "glob search", but I like the idea of implementing it in . One thing I know, is that I don't expect to read like , so os.SEP would be clearer as os.PathSeparator, or even better, as path.Separator. Now I do have a tendency to go by naming standards, but really, anything other than a YELLCASE SEP would be cleaner and clearer.

Even the community seems to agree that using os.path.sep over os.sep makes for clearer code - I'd use os.path.sep to make it very clear that it's the path separator and I recommend you use os.path.sep for clarity, since it's a path separator, not an OS separator. So I'd go with path.Separator, or path.Sep to maintain Pythonish naming.

Looking at your linked question, I see @RubberDuck already recommended using better names for your constants; sorry to reiterate the same advice here.

Underscores

I know loves snake_case, but in that kind of casing is potentially confusing (for class methods, more specifically) - I'd recommend sticking to language's PascalCase naming convention, and using a consistent camelCase for locals and parameters, for readability's sake (the actual VB/VBA conventions would go PascalCase all the way, but I find that maddening). I find pat_list would really benefit from simply being called patterns, or more accurately, patternParts - the _list suffix feels Hungarian, and seems to reflect the type, which could just as well be a regular Collection - I like that you're using your own List type though.


Parameters

The documentation I've found actually calls its sole parameter pathname:

glob.glob(pathname)

Return a possibly-empty list of path names that match pathname, which must be a string containing a path specification. pathname can be either absolute (like /usr/src/Python-1.5/Makefile) or relative (like ../../Tools/*/*.gif), and can contain shell-style wildcards.

You haven't shown us how the client code might use this function, and it's.. pretty hard to guess just by looking at this post, without diving into the os implementation - I'd have to jump to the definition of os.SubItems to figure out why I need to pass a root parameter on top of a pattern.

Looking at your added example call, I think it could be worth combining the two parameters into a single pathName parameter, making the function callable like this:

?Glob("C:\users\ptwales\????-herp\*\*.txt").ToString

The difficulty would be to parse the string and differenciate the root from the pattern - splitting on path separator and using anything to the left of a part that contains any wildcard as the root, and everything else as the pattern, would certainly be possible, and would make it a better API IMO; the root and pattern parts are really two parts of the same string, that you've split in two because it's more convenient to do so since the implementation calls os.SubItems which wants a root parameter - in other words, splitting that string in two is really only leaking implementation details into the API.


Function as a local variable

I don't like this:

Set GlobRecurse = New List
...
GlobRecurse.Extend GlobRecurse(folder, pat_list, index + 1)

While this certainly works, it is rather ugly. Not because of multiple assignments (roughly requivalent to multiple [non-returning] return statements), but because the function's name is being used a local List variable, happily being appended to.

I would much rather see this:

Private Function GlobRecurse(ByVal root As String, ByVal pat_list As List, ByVal index As Integer) As List

    Dim result As List

    If index = pat_list.Count Then
        Set result = os.SubItems(root, pat_list(index))
    Else

        Set result = New List

        Dim folder As Variant
        For Each folder In os.SubFolders(root, pat_list(index))
            result.Extend GlobRecurse(folder, pat_list, index + 1)
        Next folder

    End If

    Set GlobRecurse = result

End Function

Especially with the recursive nature of the function, you'll want to avoid repeating the function's name for no reason. My rule of thumb, is to never read from the function identifier, only to assign it.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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