I have a moderate size sub-procedure where I am calling multiple functions to feed values into other functions. I feel there may be a better or more concise way to present and run this module, but have refactored the code as far as I can see.
Public Sub CompareArrays()
Dim matchCell As Range
Set matchCell = ThisWorkbook.Sheet1.Range("A1")
Dim matchArray As Variant
matchArray = RangeToLastRow(matchCell)
Dim compareCell As Range
Set compareCell = matchCell.Parent.Range("B1")
Dim compareArray As Variant
compareArray = RangeToLastRow(compareCell)
Dim arrayToPaste As Variant
arrayToPaste = MatchArrays(matchArray, compareArray)
RangeToLastRow(matchCell.Parent.Range("C1")) = arrayToPaste
End Sub
1 Answer 1
My solution comes from this post where specifically the answer suggests a function to make an array. That seemed odd to me because it is a very simple matter to define an array from a range, but then the below occurred to me.
Dim myArray As Variant
myArray = Range("A1:A10")
Dim arrayToPaste As Variant
arrayToPaste = MatchArrays(myArray, etc)
Could also be written as:
Dim arrayToPaste As Variant
arrayToPaste = MatchArrays(MakeArray("A1:A10"), etc)
Simply by passing the function as an argument I have removed two additional lines of code and it is still clear what is happening.
Going a step further, we can also integrate the RangeToLastRow function in the OP and finish off the Sub.
Public Sub CompareArrays()
Dim arrayToPaste As Variant
arrayToPaste = MatchArrays( _
MakeArray(RangeToLastRow(ThisWorkbook.Sheet1.Range("A1")), _
MakeArray(RangeToLastRow(ThisWorkbook.Sheet1.Range("B1"))
RangeToLastRow(ThisWorkbook.Sheet1.Range("C1")) = arrayToPaste
End Sub
As long as we use descriptive function names, it should still be clear what we are doing to the inputs without having to Dim a whole lot of extra variables.
Edit:
Another method is to move the functions out of the arguments and into the function that is called, for example:
Public Sub CompareArrays()
Dim arrayToPaste As Variant
arrayToPaste = MatchArrays( _
RangeToLastRow(ThisWorkbook.Sheet1.Range("A1"), _
RangeToLastRow(ThisWorkbook.Sheet1.Range("B1"))
RangeToLastRow(ThisWorkbook.Sheet1.Range("C1")) = arrayToPaste
End Sub
Sub MatchArrays(ByRef MatchRange As Range, ByRef CompareRange As Range)
matchArray = MakeArray(MatchRange)
compareArray = MakeArray(CompareRange)
'*do stuff '
End Sub
-
1\$\begingroup\$ Careful with inlining: if an error occurs in
RangeToLastRow
it will be harder to debug an instruction with 7 statements than one instruction that does fewer things. IMO an instruction that spans 3 lines is a bit too much. The number of lines of code isn't a measure of anything other than... the number of lines of code. \$\endgroup\$Mathieu Guindon– Mathieu Guindon2016年10月06日 23:32:08 +00:00Commented Oct 6, 2016 at 23:32 -
1\$\begingroup\$ Personally, I prefer to split operations up into atomic parts even when they could be put in one function call. At the end of the day, once you've abstracted away some operations into a method, you're never going to care if it's overly verbose, but you're going to be really thankful when something breaks and you need to debug it. \$\endgroup\$Kaz– Kaz2016年10月06日 23:57:57 +00:00Commented Oct 6, 2016 at 23:57