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

ExcelVBA loop exports data from spreadsheet to table within Excel

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

Problem

I want to nest a for loop to cycle through the Range.Value but ("B4") throws me off and this can't be avoided. I was considering using an array to do this but can't seem to get the format correct. "Q3" is a spreadsheet and "Bac Form" is the attached table imported from a PDF that must be kept. This code works wonderfully - but it is clunky! I have 3 other unique PDFs with which to complete a similar task and rather than just forcing this thing to work would like to use this opportunity to improve my VBA for-looping skills.

```
Private Sub CommandButton1_Click()

Dim lr As Long
Dim FolderPath As String

FolderPath = "C:\Users\Joe.Dimaggio\Desktop\PDFs"

lr = Cells(Rows.Count, 1).End(xlUp).Row
lc = Cells(1, Columns.Count).End(xlToLeft).Column

For x = 2 To lr
Sheets("Bac Form").Range("A2").Value = _
Sheets("Q3").Cells(1, 1).Value & Sheets("Q3").Cells(x, 1).Value
Sheets("Bac Form").Range("A3").Value = _
Sheets("Q3").Cells(1, 2).Value & Sheets("Q3").Cells(x, 2).Value & _
" (Third Bacterial Quarter)"
Sheets("Bac Form").Range("A4").Value = _
Sheets("Q3").Cells(1, 3).Value & Sheets("Q3").Cells(x, 3).Value
Sheets("Bac Form").Range("B4").Value = _
Sheets("Q3").Cells(1, 4).Value & Sheets("Q3").Cells(x, 4).Value
Sheets("Bac Form").Range("A5").Value = _
Sheets("Q3").Cells(1, 5).Value & Sheets("Q3").Cells(x, 5).Value
Sheets("Bac Form").Range("A6").Value = _
Sheets("Q3").Cells(1, 6).Value & Sheets("Q3").Cells(x, 6).Value
Sheets("Bac Form").Range("A7").Value = _
Sheets("Q3").Cells(1, 7).Value & Sheets("Q3").Cells(x, 7).Value
Sheets("Bac Form").Range("A8").Value = _
Sheets("Q3").Cells(1, 8).Value & Sheets("Q3").Cells(x, 8).Value
Sheets("Bac Form").Range("A9").Value = _
Sheets("Q3").Cells(1, 9).Value & Sheets("Q3").Cells(x, 9).Value
Sheets("Bac Form").Range("A10").Value = _
Sheets("Q3").Cells(1, 10).Value & Sheets("Q3").Cells(x, 10).Value
Sheets("Bac Form").Range("A11").Value = _
Sheet

Solution

I'm having difficulty in visualising your data here, a screenshot would be really useful.

In the meantime:

Option Explicit

This should be at the top of every code module you ever create in VBA. Go to Tools --> Options --> Require Variable Declaration and it will automatically insert it for you from now on.

This is important because without it, VBA will interpret any new variable names (including mis-spellings) as entirely new variables, instead of what you intended.

It also forces you to declare your variables. So you must explicitly give them a type (Long, String, Variant etc.) and a scope (Procedure Dim var As, Module Private Var As, Project Public Var As).

This will then automatically catch all sorts of unintended situations (such as accidentally setting a number equal to an object) which would not be caught if VBA has to assume that all your variables are Variant because you never explicitly declared them.

Magic Variables

And not the good kind of magic either. A Magic Variable is any value which is hardcoded. Especially if it's hardcoded in multiple places.


lr = Cells(Rows.Count, 1).End(xlUp).Row

Why 1? How was that number determined?


"Bac Form", "A1", "Q3" etc.

Why those values in particular? If you can't look at a variable and know what it represents, then it has appeared in your code as if by magic.

If you have to hardcode values, then they should be hardcoded precisely once, into a descriptively named variable.

This:


FolderPath = "C:\Users\Joe.Dimaggio\Desktop\PDFs"

is the right way to hardcode a value. It's written once, and I can see FolderPath later on in the code and know exactly what it is.

This:

For x = 2 To lr
    Sheets("Bac Form").Range("A2").Value = _
    Sheets("Q3").Cells(1, 1).Value & Sheets("Q3").Cells(x, 1).Value
    Sheets("Bac Form").Range("A3").Value = _
    Sheets("Q3").Cells(1, 2).Value & Sheets("Q3").Cells(x, 2).Value & _
    " (Third Bacterial Quarter)"
    Sheets("Bac Form").Range("A4").Value = _
    Sheets("Q3").Cells(1, 3).Value & Sheets("Q3").Cells(x, 3).Value
    Sheets("Bac Form").Range("B4").Value = _
    Sheets("Q3").Cells(1, 4).Value & Sheets("Q3").Cells(x, 4).Value
    Sheets("Bac Form").Range("A5").Value = _
    Sheets("Q3").Cells(1, 5).Value & Sheets("Q3").Cells(x, 5).Value
    Sheets("Bac Form").Range("A6").Value = _
    Sheets("Q3").Cells(1, 6).Value & Sheets("Q3").Cells(x, 6).Value
    Sheets("Bac Form").Range("A7").Value = _
    Sheets("Q3").Cells(1, 7).Value & Sheets("Q3").Cells(x, 7).Value
    Sheets("Bac Form").Range("A8").Value = _
    Sheets("Q3").Cells(1, 8).Value & Sheets("Q3").Cells(x, 8).Value
    Sheets("Bac Form").Range("A9").Value = _
    Sheets("Q3").Cells(1, 9).Value & Sheets("Q3").Cells(x, 9).Value
    Sheets("Bac Form").Range("A10").Value = _
    Sheets("Q3").Cells(1, 10).Value & Sheets("Q3").Cells(x, 10).Value


is not. What happens if the name of your sheet changes? Or your data table moves because someone inserted/deleted a column/row? You'll have to go and re-code every single value.

Naming

Variable names should be Concise, Descriptive and, above all, Unambiguous.


FolderPath

is a good name because it's clear exactly what the variable contains/represents.


lr

is not. If I encounter that line halfway through your code, I'm going to go all the way back through to remind myself what it's meant to be representing. Just call it lastRow and you avoid all that trouble.

VBA Naming Conventions:

Typical VBA Naming conventions go as follows:

camelCase for local variables.

PascalCase for Module/Global Variables.

SHOUTY_SNAKE_CASE for constants.

Get in the habit of following them.

Bringing it all together

Step 1, put your worksheets in variables:

For x = 2 To lr
    wsBacForm.Range("A2").Value = wsQ3.Cells(1, 1).Value & wsQ3.Cells(x, 1).Value
    wsBacForm.Range("A3").Value = wsQ3.Cells(1, 2).Value & wsQ3.Cells(x, 2).Value & " (Third Bacterial Quarter)"
    wsBacForm.Range("A4").Value = wsQ3.Cells(1, 3).Value & wsQ3.Cells(x, 3).Value
    wsBacForm.Range("B4").Value = wsQ3.Cells(1, 4).Value & wsQ3.Cells(x, 4).Value
    wsBacForm.Range("A5").Value = wsQ3.Cells(1, 5).Value & wsQ3.Cells(x, 5).Value
    wsBacForm.Range("A6").Value = wsQ3.Cells(1, 6).Value & wsQ3.Cells(x, 6).Value
    wsBacForm.Range("A7").Value = wsQ3.Cells(1, 7).Value & wsQ3.Cells(x, 7).Value
    wsBacForm.Range("A8").Value = wsQ3.Cells(1, 8).Value & wsQ3.Cells(x, 8).Value
    wsBacForm.Range("A9").Value = wsQ3.Cells(1, 9).Value & wsQ3.Cells(x, 9).Value
    wsBacForm.Range("A10").Value = wsQ3.Cells(1, 10).Value & wsQ3.Cells(x, 10).Value


And suddenly the structure becomes clear.

Then:

Add options to change the row/column position of your tables.

Loop through rows/columns.

Add separate variables for each sheet to deal with (potentially) separate positions.

Add an index counter to track where to insert extra text.

Put the filename creation on a separate line.

Add proper Range objects.

And now, if you

Code Snippets

For x = 2 To lr
    Sheets("Bac Form").Range("A2").Value = _
    Sheets("Q3").Cells(1, 1).Value & Sheets("Q3").Cells(x, 1).Value
    Sheets("Bac Form").Range("A3").Value = _
    Sheets("Q3").Cells(1, 2).Value & Sheets("Q3").Cells(x, 2).Value & _
    " (Third Bacterial Quarter)"
    Sheets("Bac Form").Range("A4").Value = _
    Sheets("Q3").Cells(1, 3).Value & Sheets("Q3").Cells(x, 3).Value
    Sheets("Bac Form").Range("B4").Value = _
    Sheets("Q3").Cells(1, 4).Value & Sheets("Q3").Cells(x, 4).Value
    Sheets("Bac Form").Range("A5").Value = _
    Sheets("Q3").Cells(1, 5).Value & Sheets("Q3").Cells(x, 5).Value
    Sheets("Bac Form").Range("A6").Value = _
    Sheets("Q3").Cells(1, 6).Value & Sheets("Q3").Cells(x, 6).Value
    Sheets("Bac Form").Range("A7").Value = _
    Sheets("Q3").Cells(1, 7).Value & Sheets("Q3").Cells(x, 7).Value
    Sheets("Bac Form").Range("A8").Value = _
    Sheets("Q3").Cells(1, 8).Value & Sheets("Q3").Cells(x, 8).Value
    Sheets("Bac Form").Range("A9").Value = _
    Sheets("Q3").Cells(1, 9).Value & Sheets("Q3").Cells(x, 9).Value
    Sheets("Bac Form").Range("A10").Value = _
    Sheets("Q3").Cells(1, 10).Value & Sheets("Q3").Cells(x, 10).Value
For x = 2 To lr
    wsBacForm.Range("A2").Value = wsQ3.Cells(1, 1).Value & wsQ3.Cells(x, 1).Value
    wsBacForm.Range("A3").Value = wsQ3.Cells(1, 2).Value & wsQ3.Cells(x, 2).Value & " (Third Bacterial Quarter)"
    wsBacForm.Range("A4").Value = wsQ3.Cells(1, 3).Value & wsQ3.Cells(x, 3).Value
    wsBacForm.Range("B4").Value = wsQ3.Cells(1, 4).Value & wsQ3.Cells(x, 4).Value
    wsBacForm.Range("A5").Value = wsQ3.Cells(1, 5).Value & wsQ3.Cells(x, 5).Value
    wsBacForm.Range("A6").Value = wsQ3.Cells(1, 6).Value & wsQ3.Cells(x, 6).Value
    wsBacForm.Range("A7").Value = wsQ3.Cells(1, 7).Value & wsQ3.Cells(x, 7).Value
    wsBacForm.Range("A8").Value = wsQ3.Cells(1, 8).Value & wsQ3.Cells(x, 8).Value
    wsBacForm.Range("A9").Value = wsQ3.Cells(1, 9).Value & wsQ3.Cells(x, 9).Value
    wsBacForm.Range("A10").Value = wsQ3.Cells(1, 10).Value & wsQ3.Cells(x, 10).Value
Private Sub CommandButton1_Click()

    Dim fileName As String

    Const FOLDER_PATH As String = "C:\Users\Joe.Dimaggio\Desktop\PDFs"

    Dim wsBacForm As Worksheet, wsQ3 As Worksheet
    Set wsBacForm = Sheets("Bac Form")
    Set wsQ3 = Sheets("Q3")

    Dim q3BaseRow As Long, q3BaseCol As Long '/ Location of the Q3 data table
    q3BaseRow = 1
    q3BaseCol = 1

    Dim bacBaseRow As Long, bacBaseCol As Long '/ Location of the bac output table
    bacBaseRow = 1
    bacBaseCol = 1

    Dim lastRow As Long, lastCol As Long
    lastRow = wsQ3.Cells(Rows.Count, q3BaseCol).End(xlUp).row
    lastCol = wsQ3.Cells(q3BaseRow, Columns.Count).End(xlToLeft).Column

    Dim bacRow As Long, bacCol As Long, q3Row As Long, q3Col As Long
    Dim row As Long, col As Long, counter As Long

    Dim bacOutputCell As Range, q3HeaderCell As Range, q3DataCell As Range
    Dim outputString As String

    For row = q3BaseRow To lastRow
        bacRow = row - q3BaseRow + bacBaseRow
        q3Row = row

        counter = 0
        For col = q3BaseCol To lastCol
            counter = counter + 1
            q3Col = col

            Set bacOutputCell = wsBacForm.Cells(bacRow, bacBaseCol)
            Set q3HeaderCell = wsQ3.Cells(q3BaseRow, q3Col)
            Set q3DataCell = wsQ3.Cells(q3Row, q3Col)

            outputString = q3HeaderCell.Value & q3DataCell.Value

            Select Case counter '/ used a counter so that it is (absolute position of column) agnostic

                Case Is = 2
                outputString = outputString & " (Third Bacterial Quarter)"

                Case Is = 14
                outputString = outputString & " colony/100 ml"

                Case Is = 16
                outputString = outputString & " MPN/100 ml"

            End Select

            bacOutputCell.Value = outputString
        Next col

        fileName = FOLDER_PATH & "\" & wsBacForm.Name & "(" & wsQ3.Name & ")" & (row - 1)
        wsBacForm.ExportAsFixedFormat Type:=xlTypePDF, fileName:=fileName, openafterpublish:=False
    Next row

End Sub

Context

StackExchange Code Review Q#116695, answer score: 8

Revisions (0)

No revisions yet.