Converting Data in Rows and Columns to Rows in VBA for Excel

I have a working VBA script for Excel that converts a matrix of data with multiple records in per row to multiple rows with one record per row.

A StackOverflow user told me that the code could use significant improvement, specifically mentioning implicit variants (not quite sure where I went wrong there), difficult to read code, splitting responsibilities and something about GoTo, the 1980's and raptors...

The script takes data like this:

Materials       Person1     Person2
---------       ---------   ---------
563718          20          40
837563          15          35

And can convert it to this:

Person          Materials   Data
---------       ---------   ---------
Person1         563718      20
Person1         837563      15
Person2         563718      40
Person2         837563      35

Data is supplied by a third party. Each record/transaction (ex. quantity purchased, for each materials type, by customer) needs to be formatted in a separate row.

The script asked the the user about this data. In this example the user specifies 1 "Header Column" (one column beginning on the left that will remain as is). Then gives a name of "Person" for a new field made from values in the headers of the remaining columns to the right. The values (amounts) below these headers are also made into a new field, called "Data" by default.

I am open to any advice, but I am most interested in (1) writing better code in general, and (2) making this script adaptable and easier for others to use.

The script below was originally written by Peter T Oboyski. I extensively modified it.

```
Option Explicit

Sub MatrixConverter2_3()

'--------------------------------------------------
' This section declares variables for use in the script

Dim book, head, cels, mtrx, dbase, v, UserReady, columnsToCombine, RowName, DefaultRowName, DefaultColName1, DefaultColName2, ColName As String
Dim defaultHeaderRows, defaultHeaderColumns, c, r, selectionCols, ro, col, newro, n

Declaring variables at the top of a procedure was a recommended practice in 90's VB code, because "it makes it easy to see everything that the procedure needs at once". When a procedure would fit on a single screen, it wasn't too bad - but when a procedure scrolls several screens down, and uses 30-40 local variables, that "wall of declarations" actually made it harder to read (/maintain) the code, because you'd constantly need to scroll back up to see the declaration of a given variable, and then you'd waste considerable time locating which line you were looking at on your way back down. Been there, done that.

So to avoid the "wall of declarations" you could make a single instruction to declare a list of variables, like this:

Dim book, head, cels, mtrx, dbase, v, UserReady, columnsToCombine, RowName, DefaultRowName, DefaultColName1, DefaultColName2, ColName As String
Dim defaultHeaderRows, defaultHeaderColumns, c, r, selectionCols, ro, col, newro, newcol, rotot, coltot, all, rowz, colz, tot As Long

There's a trap though: Dim foo, bar, baz As String declares the last one (baz) as a String, and leaves foo and bar implicitly Variant - which incurs useless overhead and requires more storage/memory than needed (not that that is a problem nowadays though).

'--------------------------------------------------
' This section declares variables for use in the script

Dim book
Dim head
Dim cels
Dim mtrx
Dim dbase
Dim v
Dim UserReady
Dim columnsToCombine
Dim RowName
Dim DefaultRowName
Dim DefaultColName1
Dim DefaultColName2
Dim ColName As String

Dim defaultHeaderRows
Dim defaultHeaderColumns
Dim c
Dim r
Dim selectionCols
Dim ro
Dim col
Dim newro
Dim newcol
Dim rotot
Dim coltot
Dim all
Dim rowz
Dim colz
Dim tot As Long

Dim headers(100) As Variant
Dim dun As Boolean

Fun fact: the first three variables are actually Strings that are concatenated into a message for a MsgBox that's displayed at the very end of the procedure.

So we have that wall of declarations, and that banner comment telling us that we're looking at a wall of declarations. Comments shouldn't state the obvious like that; good comments tell us what the code can't say all by itself: it tells us why the code does something.

But back at these variables: DefaultColName1, DefaultColName2 and defaultHeaderRows are never never used! Actually DefaultColName1 and DefaultColName2 aren't even assigned, and never referred to, not even in dead/commented-out code - but who could have known? That's why declaring variables closer to where they're used is a much better practice: no wall of declarations, and it's much harder to declare a variable that's left unused, without noticing.

'--------------------------------------------------
' This section asks about data types, row headers, and column headers

In other words, this section is collecting user input - it should be a separate procedure!

UserReady = MsgBox("Have you selected the entire data set (not the column headers) to be converted?", vbYesNoCancel)
If UserReady = vbNo Or UserReady = vbCancel Then GoTo EndMatrixMacro

That UserReady variable should have been declared like this:

Dim UserReady As VbMsgBoxResult

Actually, since the only thing we're using it for is effectively to cancel the whole thing, might as well not declare it at all and do this instead:

If MsgBox("Have you selected the entire data set (not the column headers) to be converted?", vbYesNoCancel) <> vbYes Then Exit Sub

...And we just eliminated a GoTo jump!

all = MsgBox("Exclude zeros and empty cells?", vbYesNoCancel)
If all = vbCancel Then GoTo EndMatrixMacro

Same thing here: all should have been declared As VbMsgBoxResult, and there's no need to GoTo EndMatrixMacro either. The name isn't ideal, too: vbYes stands for "exclude zeros and empty cells", and vbNo stands for "include zeros and empty cells" - which means the true meaning of all is the exact opposite of what it appears to be! I'd rename it to IsExcludingZeroAndEmpty, and declare it As Boolean, because we don't really care about the MsgBox result here, all that matters is whether or not we're to include zeros and empty values.

That all variable is used here:

If ((Sheets(mtrx).Cells(ro, col) <> 0) Or (all <> 6)) Then

What's that magic number 6? If the variable would have been declared As VbMsgBoxResult, the VBE's IntelliSense would have suggested to use vbYes instead of its underlying numeric value. But that's all moot with a proper Boolean:

If Sheets(mtrx).Cells(ro, col) <> 0 Or Not IsExcludingZeroAndEmpty Then

(note, I might have gotten confused with the reversed "all" logic here... but you get the point I'm sure - which one is easier to understand?)

Next the script prompts for how many header columns we're looking at:

```
colz = InputBox("How many HEADER COLUMNS?" & vbNewLine & vbNewLine & "(These are the columns on the left side of your data

StackExchange Code Review Q#139718, answer score: 5