HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Defining array contents using multiple statements on one line

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
linearraystatementscontentsoneusingmultipledefining

Problem

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"

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#102423, answer score: 5

Revisions (0)

No revisions yet.