1

I want to create a summary table in a new sheet, and at the moment I'm just doing it very crudely. I will try a more elegant solution in the future.

Anyway, this is the code I have so far:

Sub createsummarytable()

Worksheets.Add().Name = "datasummary"

With Worksheets("datasummary")

Dim i As Long
Dim Startpoint As Long

Startpoint = -5

For i = 1 To 40
.Cells(Startpoint + (5 * i), 1).Value = "Block" & "i"
Next i

End With
End Sub

I am getting the error in the title on line: .Cells(Startpoint + (5 * i), 1).Value = "Block" & "i"

If anyone wants to make the code more elegant in addition to solving the error, that would be appreciated.

shecodes
  • 111
  • 9

3 Answers3

2

Off-by-one. There is no column/row 0 in Excel; -5 + (5 * 1) evaluates to 0:

.Cells(0, 1).Value = 42 'same error

You need to adjust by +1:

For i = 1 To 40
    .Cells(Startpoint + (5 * i) + 1, 1).Value = "Block" & i
Next i

If your code works as intended, describe your working code in a new question on Code Review.

Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
  • 2
    Mat's Mug's answer should resolve your error. It may be worth mentioning also that if your intent is to use your counter, i, such that your output would be Block 1, Block 2, Block 3, etc, then you would need to modify your code to: `.Cells(Startpoint+(5*i)+1,1).Value="Block " & i`, else you'll repeatedly get Blocki,Blocki,Blocki – kpg987 Nov 01 '16 at 03:59
  • Argh what a frustratingly silly mistake! Thanks Mat's Mug, that works. Why do you want me to describe my code in Code Review SE? Thanks @kpg987 for that catch. I would have found that error myself, had my code iterated that far. – shecodes Nov 02 '16 at 07:59
0

@Mat's Mug picked up on the off-by-one error, and @kpg987 got your use of "i" as a string-literal instead of a variable, but your code can be improved.

Here are some of the changes I made:

  • Scope the procedure to Private or Public

  • Use meaningful names

  • Use Constants to determine the starting row and number of each rows for each block. StartingPoint = -5 isn't useful, whereas START_ROW = 1 is quite clear about what it is and where it will actually start.

  • Adjust the formula to use the constants.

  • Refer to the target workbook explicitly, otherwise the active workbook will be used.

  • Set a reference to the added worksheet when using the add command.

  • Use the strongly typed reference with the With command, because With Worksheets("datasummary") will be late bound)

  • Use camelCasing for variable names

Result:

Private Sub createSummaryTable()

    Const WORKSHEET_NAME As String = "datasummary"
    Const START_ROW As Long = 1
    Const BLOCK_COLUMN as Long = 1
    Const BLOCK_SIZE As Long = 5
    Const BLOCK_COUNT As Long = 40
    Const BLOCK_PREFIX As String = "Block"

    Dim dataSummary As Worksheet
    Set dataSummary = ThisWorkbook.Worksheets.Add
    With dataSummary
        .Name = WORKSHEET_NAME
        Dim blockCounter As Long
        For blockCounter = 1 To BLOCK_COUNT
            .Cells(START_ROW + (BLOCK_SIZE * (blockCounter - 1)), BLOCK_COLUMN).Value = BLOCK_PREFIX & blockCounter
        Next blockCounter
    End With
End Sub
ThunderFrame
  • 9,352
  • 2
  • 29
  • 60
  • whaat is the reason for scoping to private or public? The purpose of the macro is to draw up a table to summarise the data from [http://stackoverflow.com/questions/40147555/using-a-dictionary-and-array-to-count-cell-values]. As such, "Block" refers to a specific set of data, which I will eventually need to expand upon in the current macro with "trial", "AOI entries", and "RT". Also, what does camelcasing mean? – shecodes Nov 02 '16 at 08:30
  • `Public` procedures can be called by *any* other project, and from any module inside the *same* project. `Private` procedures are only callable by code in the *same module in the same project*. It is better practice to make everything `Private` *unless* it *needs* to be exposed. If you don't specify `Public` or `Private`, the VBE implicitly defaults the treatment to `Public`, which is the *opposite* of what you want. If you need a procedure to be available across all modules *within* your project, you mark the procedure as `Public` but mark the module as `Option Private Module`. – ThunderFrame Nov 03 '16 at 22:25
0

a "no-loop" approach:

Private Sub createSummaryTable2()
    Dim dataSummary As Worksheet
    With ThisWorkbook.Worksheets.Add
        .Name = "datasummary"
        With .Range("A1").Resize((40 - 1) * 5 + 1)
            .FormulaR1C1 = "=IF(MOD(ROW(),5)=1,""Block"" & int(ROW()/5)+1,"""")"
            .Value = .Value
        End With
    End With
End Sub
user3598756
  • 28,893
  • 4
  • 18
  • 28