3
\$\begingroup\$

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
200_success
146k22 gold badges190 silver badges478 bronze badges
asked Oct 6, 2016 at 22:35
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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
answered Oct 6, 2016 at 22:35
\$\endgroup\$
2
  • 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\$ Commented 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\$ Commented Oct 6, 2016 at 23:57

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.