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

Get Worksheet Data Array (Standard Methods)

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

Problem

I'm re-writing my module of Standard Methods. Virtually every project I do begins with grabbing some number of Data Tables and putting them in arrays. So, this is my general "Get Worksheet Data" method(s).

As always, particularly interested in maintainability, but all feedback is welcome.

Public Function GetWsDataArray(ByRef wbTarget As Workbook, ByRef wsTarget As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
                            , Optional ByVal searchStartRow As Long = 1, Optional ByVal searchStartColumn As Long = 1 _
                            , Optional ByVal searchEndRow As Long = 10, Optional ByVal searchEndColumn As Long = 10) As Variant
                            '/ 10x10 is arbitrary search range that should cover almost all typical worksheets

    Dim dataArray As Variant
        dataArray = Array()
        dataArray = GetWsDataRange(wbTarget, wsTarget, topLeftCellText, useCurrentRegion, searchStartRow, searchStartColumn, searchEndRow, searchEndColumn)

        GetWsDataArray = dataArray

End Function


```
Public Function GetWsDataRange(ByRef wbTarget As Workbook, ByRef wsTarget As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
, ByVal searchStartRow As Long, ByVal searchStartColumn As Long _
, ByVal searchEndRow As Long, ByVal searchEndColumn As Long) As Range

Dim wbSource As Workbook, wsSource As Worksheet
Set wbSource = ActiveWorkbook
Set wsSource = ActiveSheet

wbTarget.Activate
wsTarget.Activate
UnhideWsCellsAndRemoveFilters wsTarget

Dim topLeftCell As Range, searchRange As Range, dataRange As Range
Set searchRange = wsTarget.Range(Cells(searchStartRow, searchStartColumn), Cells(searchEndRow, searchEndColumn))
Set topLeftCell = CellContainingStringInRange(searchRange, topLeftCellText)

Dim lastRow As Long, lastCol As Long
If useCurrentRegion Then
Set dataRang

Solution

UnhideWsCellsAndRemoveFilters()

Well thats a real strange name. Wouldn't it be better to replace Unhide with Show? Like ShowWsCellsAndRemoveFilters() ?

CellContainingStringInRange()

You should declare your variables as near as possible to their usage. This together with reverting the if condition to return early would lead to

Public Function CellContainingStringInRange(ByRef rngSearch As Range, ByVal strSearch As String) As Range

    Set CellContainingStringInRange = rngSearch.Find(strSearch, LookIn:=xlValues)
    If CellContainingStringInRange IsNot Nothing Then
        Exit Function
    End If

    Dim errorMessage As String
    errorMessage = "Couldn't find cell """ & strSearch & """ in " & rngSearch.Worksheet.name
    PrintErrorMessage errorMessage, stopExecution:=True
End Function


GetWsDataRange()

At first glance at this

Dim wbSource As Workbook, wsSource As Worksheet
Set wbSource = ActiveWorkbook
Set wsSource = ActiveSheet


it will be overseen that wbSource and wsSource are different objects. You should consider to use some vertical spacing to separate these and/or to use different names like sourceBook and sourceSheet. The same is true for the method arguments wbTarget and wsTarget.

Generally declaring multiple variables on the same line should be avoided because it is much harder to read/maintain.

Having the dataRange As Range doesn't provide any value because it is only used "once". You can assign the return value directly.

Implementing this points and having the declarations as near to their usage as possible and on separate lines will lead to

Public Function GetWsDataRange(ByRef targetBook As Workbook, ByRef targetSheet As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
                            , ByVal searchStartRow As Long, ByVal searchStartColumn As Long _
                            , ByVal searchEndRow As Long, ByVal searchEndColumn As Long) As Range

    Dim sourceBook As Workbook
    Set sourceBook = ActiveWorkbook

    Dim sourceSheet As Worksheet
    Set sourceSheet = ActiveSheet

    targetBook.Activate
    targetSheet.Activate
    ShowWsCellsAndRemoveFilters ws

    Dim searchRange As Range
    Set searchRange = targetSheet.Range(Cells(searchStartRow, searchStartColumn), Cells(searchEndRow, searchEndColumn))

    Dim topLeftCell As Range
    Set topLeftCell = CellContainingStringInRange(searchRange, topLeftCellText)

    Dim dataRange As Range

    If useCurrentRegion Then
        Set GetWsDataRange = topLeftCell.CurrentRegion
    Else
        Dim lastRow As Long
        lastRow = Cells(Rows.Count, topLeftCell.Column).End(xlUp).row

        Dim lastCol As Long
        lastCol = Cells(topLeftCell.row, Columns.Count).End(xlToLeft).Column

        Set GetWsDataRange = targetSheet.Range(topLeftCell, Cells(lastRow, lastCol))
    End If

    sourceBook.Activate
    sourceSheet.Activate

End Function

Code Snippets

Public Function CellContainingStringInRange(ByRef rngSearch As Range, ByVal strSearch As String) As Range

    Set CellContainingStringInRange = rngSearch.Find(strSearch, LookIn:=xlValues)
    If CellContainingStringInRange IsNot Nothing Then
        Exit Function
    End If

    Dim errorMessage As String
    errorMessage = "Couldn't find cell """ & strSearch & """ in " & rngSearch.Worksheet.name
    PrintErrorMessage errorMessage, stopExecution:=True
End Function
Dim wbSource As Workbook, wsSource As Worksheet
Set wbSource = ActiveWorkbook
Set wsSource = ActiveSheet
Public Function GetWsDataRange(ByRef targetBook As Workbook, ByRef targetSheet As Worksheet, ByVal topLeftCellText As String, ByVal useCurrentRegion As Boolean _
                            , ByVal searchStartRow As Long, ByVal searchStartColumn As Long _
                            , ByVal searchEndRow As Long, ByVal searchEndColumn As Long) As Range

    Dim sourceBook As Workbook
    Set sourceBook = ActiveWorkbook

    Dim sourceSheet As Worksheet
    Set sourceSheet = ActiveSheet

    targetBook.Activate
    targetSheet.Activate
    ShowWsCellsAndRemoveFilters ws

    Dim searchRange As Range
    Set searchRange = targetSheet.Range(Cells(searchStartRow, searchStartColumn), Cells(searchEndRow, searchEndColumn))

    Dim topLeftCell As Range
    Set topLeftCell = CellContainingStringInRange(searchRange, topLeftCellText)

    Dim dataRange As Range

    If useCurrentRegion Then
        Set GetWsDataRange = topLeftCell.CurrentRegion
    Else
        Dim lastRow As Long
        lastRow = Cells(Rows.Count, topLeftCell.Column).End(xlUp).row

        Dim lastCol As Long
        lastCol = Cells(topLeftCell.row, Columns.Count).End(xlToLeft).Column

        Set GetWsDataRange = targetSheet.Range(topLeftCell, Cells(lastRow, lastCol))
    End If

    sourceBook.Activate
    sourceSheet.Activate

End Function

Context

StackExchange Code Review Q#117145, answer score: 5

Revisions (0)

No revisions yet.