patternMinor
Get Worksheet Data Array (Standard Methods)
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 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
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 FunctionGetWsDataRange()At first glance at this
Dim wbSource As Workbook, wsSource As Worksheet
Set wbSource = ActiveWorkbook
Set wsSource = ActiveSheetit 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 FunctionCode 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 FunctionDim wbSource As Workbook, wsSource As Worksheet
Set wbSource = ActiveWorkbook
Set wsSource = ActiveSheetPublic 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 FunctionContext
StackExchange Code Review Q#117145, answer score: 5
Revisions (0)
No revisions yet.