5
\$\begingroup\$

I'm writing a sub which is going to determine which type of business is being detailed on a particular line of data. It's going to use column positions to check if the data exists, and then output the corresponding string if it does.

I've decided that the best way to organise a group of my arguments is using a 2D array. Now, for readability, clarity and general good coding practice, which is better:

This:

arrAmountColumns(1, 1) = lngInvestmentAmountColumn: arrAmountColumns(1, 2) = "Investment Amount"
arrAmountColumns(2, 1) = lngSinglePremiumColumn: arrAmountColumns(2, 2) = "Single Premium"
arrAmountColumns(3, 1) = lngMonthlyPremiumColumn: arrAmountColumns(3, 2) = "Monthly Premium"

or this:

arrAmountColumns(1, 1) = lngInvestmentAmountColumn
arrAmountColumns(1, 2) = "Investment Amount"

arrAmountColumns(2, 1) = lngSinglePremiumColumn
arrAmountColumns(2, 2) = "Single Premium"

arrAmountColumns(3, 1) = lngMonthlyPremiumColumn
arrAmountColumns(3, 2) = "Monthly Premium"
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

I'd go with the second snippet. They're instructions, and instructions read more naturally from top to bottom, rather than from left to right.

Cramming multiple instructions into a single line is a bad habit IMO - it's not because the language supports it that it's necessarily a good idea; the array has 2 columns.. what if it were 10? Would you consider cramming 10 instructions on the same line?

Your indices seem to go (row,column), which amounts to (y,x), which is a bit counter-intuitive, and if you wanted to write that array into a worksheet range, you'd have to transpose it:

Range("A1:B" & UBound(arrAmountColumns,2)) = arrAmountColumns

wouldn't produce the expected result.

By using the first index as your "column" (X) and the second as you "row" (Y), you eliminate that ambiguity and the code would read (x,y), which is a much more intuitive way of visualizing 2D arrays.

\$\endgroup\$
5
  • \$\begingroup\$ ah, Those values aren't columns per se, this was just a convenient way for storing the position of certain columns in another array, along with their associated descriptors. It felt more intuitive in my head to have: .... | ColumnPositionInArray | Desciption of Column | \$\endgroup\$
    – Kaz
    Commented Aug 31, 2015 at 18:52
  • \$\begingroup\$ You can expect an upvote just as soon as we get Reloaded :) \$\endgroup\$
    – Kaz
    Commented Aug 31, 2015 at 18:55
  • \$\begingroup\$ (this is why context matters!) In that case, I'd suggest to ditch the array and use a Scripting.Dictionary instead. \$\endgroup\$ Commented Aug 31, 2015 at 18:59
  • 1
    \$\begingroup\$ Scripting Dictionaries (along with pivot tables) are next on my list of "Learn how to use these" just as soon as my current project's completed. \$\endgroup\$
    – Kaz
    Commented Aug 31, 2015 at 19:04
  • \$\begingroup\$ I've already re-written it once in the last fortnight, my boss might actually start asking when it's going to be finished if I take much longer \$\endgroup\$
    – Kaz
    Commented Aug 31, 2015 at 19:05

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.