HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Wrapper class for the shell "dir" utility

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thedirutilityshellwrapperforclass

Problem

I was inspired to throw this class together by an SO question and thought I'd subject it to some welcome criticism here.

The class is basically just a wrapper around the shell utility dir, and is intended as a replacement for the built in Dir$ function. The rationale for replacing the function is that it has many more features than either the built in VBA functionality or the Scripting.FileSystemObject when it comes to directory listings. For example, Dir$ won't recurse subdirectories, Scripting.FileSystemObject won't take wildcards, and neither of them will filter for file attributes or sort the output. This class serves to fill that gap.

Disclaimer: This is essentially a rough draft waiting to be extended. Known limitations include:

  • It doesn't test for conflicting attribute or sort flags. The dir


utility doesn't either, but an extension should probably extend.
This gives it slightly different behaviour because the parameter
order is the tie breaker for dir.

  • It should probably have the option to specify whether or not to return the full path as opposed to the just the file. The behaviour currently depends on whether the RecurseSubDirectories property is set to True or False (just


like dir, but again the ultimate goal is to extend).

  • The command window popping up from the WScript.Shell.Exec call is annoying. This will likely require some future API hacks.



  • Other things I'm not thinking of. ;-)



All code other than the usage example is in DirectoryListing.cls.

Header: The FileAttributes and SortOrder enumerations are used as flags. TimeField values can't be combined, so they are a straight up Enum. There's probably some room for criticism here...

```
Option Explicit

Public Enum FileAttributes
Default = 0
HiddenFiles = 2 ^ 0
NonHiddenFiles = 2 ^ 1
SystemFiles = 2 ^ 2
NonSystemFiles = 2 ^ 3
DirectoriesOnly = 2 ^ 4
FilesOnly = 2 ^ 5
ArchiveReady = 2 ^ 6
NonArchiveReady = 2 ^ 7

Solution

Private Const BASE_COMMAND = "cmd /c dir "


The constant declaration doesn't have an explicit type; this would trigger an inspection result with Rubberduck; applying thhe quickfix would turn the declaration into this:

Private Const BASE_COMMAND As String = "cmd /c dir "


It's debatable whether explicitly assigning the return value has any use here:

If SortFlags = SortOrder.Default Then
    SortArguments = vbNullString
    Exit Function
End If


The result would be the same without the assignment... on the other hand, it's a good practice to have all code paths return a value, be it only for the love of explicitness.

Same here:

If AttributeFlags = SortOrder.Default Then
    AttributeArguments = vbNullString
    Exit Function
End If


...except there's a minor little copy-pasta error here - SortOrder.Default should be FileAttributes.Default! ..but it's no biggie because both values are 0.

The naming of enum types isn't consistent - flag enums should have a plural name:

Public Enum FileAttributes 'correct
    Default = 0
    HiddenFiles = 2 ^ 0
    NonHiddenFiles = 2 ^ 1
    SystemFiles = 2 ^ 2
    '...
End Enum

Public Enum SortOrder 'how does client code know they can be combined?
    Default = 0
    NameAscending = 2 ^ 0
    NameDescending = 2 ^ 1
    ExtensionAscending = 2 ^ 2
    '...
End Enum

Public Enum TimeField 'ok
    Default = 0
    Creation = 1
    LastAccess = 2
    LastWritten = 3
End Enum


I have no problem with TimeField or its values. Perhaps better names could be FileAttributeFlags and SortOrderFlags for the two flag enum types? I have no neat solution for the mutually exclusive sort flag values though, other than proper validation and error-raising in Property Let SortFlags.

The TimeField enum member values don't need to be explicit (they're assigned to the default values anyway).

SortOrder values come in mutually exclusive pairs, so there should be some validation logic in the Property Let SortFlags member, to raise an error that tells the client code when it tries to set the NameAscending flag when NameDescending is already set.

Public Property Let SortFlags(ByVal value As SortOrder)
    ThrowOnConflictingSortFlags value
    mSort = value
End Property

Private Sub ThrowOnConflictingSortFlags(ByVal value As SortOrder)
    'this is where short-circuiting logical operators would be nice...
    If HasFlag(value, NameAscending + NameDescending) Then OnConflictingFlagsError NameAscending, NameDescending
    If HasFlag(value, ExtensionAscending + ExtensionDescending) Then OnConflictingFlagsError ExtensionAscending, ExtensionDescending
    If HasFlag(value, TimeAscending + TimeDescending) Then OnConflictingFlagsError TimeAscending, TimeDescending
    If HasFlag(value, SizeAscending + SizeDescending) Then OnConflictingFlagsError SizeAscending, SizeDescending
    If HasFlag(value, DirectoriesFirst, DirectoriesLast) Then OnConflictingFlagsError DirectoriesFirst, DirectoriesLast
End Sub

Private Sub OnConflictingFlagsError(ByVal flag1 As SortOrder, ByVal flag2 As SortOrder)
    'something like this could work I guess
    Err.Raise 5, TypeName(Me), "Specified sort order flags values " & flag1 & " and " & flag2 & " are mutually exclusive."
End Sub

Private Function HasFlag(ByVal value As Long, ByVal flag As Long) As Boolean
    HasFlag = (value And flag) = flag
End Function


Such a HasFlag function could very well be reused to simplify SortArguments and AttributeArguments functions, too.

Code Snippets

Private Const BASE_COMMAND = "cmd /c dir "
Private Const BASE_COMMAND As String = "cmd /c dir "
If SortFlags = SortOrder.Default Then
    SortArguments = vbNullString
    Exit Function
End If
If AttributeFlags = SortOrder.Default Then
    AttributeArguments = vbNullString
    Exit Function
End If
Public Enum FileAttributes 'correct
    Default = 0
    HiddenFiles = 2 ^ 0
    NonHiddenFiles = 2 ^ 1
    SystemFiles = 2 ^ 2
    '...
End Enum

Public Enum SortOrder 'how does client code know they can be combined?
    Default = 0
    NameAscending = 2 ^ 0
    NameDescending = 2 ^ 1
    ExtensionAscending = 2 ^ 2
    '...
End Enum

Public Enum TimeField 'ok
    Default = 0
    Creation = 1
    LastAccess = 2
    LastWritten = 3
End Enum

Context

StackExchange Code Review Q#140179, answer score: 2

Revisions (0)

No revisions yet.