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
\$\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
\$\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\$
    – Kaz
    Commented Oct 6, 2016 at 23:57

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.