2

Firstly, I wasn't sure if VBA questions belong here or on SuperUser, apologies if in wrong place.

I am trying to teach myself VBA, and going through various exercises/challenges I've found online. I have made a macro that writes out a multiplication table, its almost certainly very inefficient, but a good starting point on writing loops etc.

My code is as follows:

Sub TimesTable()
Dim TimesTable As Integer
Dim Plot As Integer
Dim Numbers As Long
Dim RowNumbers As Long
Dim StartTime As Double
Dim SecondsElapsed As Double
'Turn off various things to speed up macro
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
'Start Timer
StartTime = Timer
ActiveSheet.Cells.ClearContents
'Ask for and read input into variable
TimesTable = InputBox("Enter a Integer to display Times Table")
'Write Axis lines
For Plot = 1 To TimesTable
ActiveSheet.Range("A1").Offset(0, Plot).Value = Plot
ActiveSheet.Range("A1").Offset(Plot, 0).Value = Plot
ActiveSheet.Range("A2").Select
Next
'Start loop 1 to kick off line writing
For RowNumbers = 1 To TimesTable
'Start loop 2 to write actual lines
    For Numbers = 1 To TimesTable
    ActiveCell.Offset(0, 1).Select
    ActiveCell.Value = RowNumbers * Numbers
    Next
ActiveCell.Offset(1, -TimesTable).Select
Next
'Turn on screen updating etc again
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic
SecondsElapsed = Round(Timer - StartTime, 2)
'Display time taken to run macro
MsgBox "This code ran successfully in " & SecondsElapsed & " seconds", vbInformation
End Sub

I've managed to get it all working and learned a few things on the way (mostly how to nest loops and that Integer has a limit), but I was wondering if there is a better way to write each calculation rather than using cell offsets? At the moment it writes the calculation, offsets by one cell to the right, and then when it gets to the end of the line, offsets back to the start of the row, and then goes down one row. I've read that for speed you want to avoid referencing specific cells when possible but struggling with how I would achieve this.

Community
  • 1
  • 1
Dekks
  • 137
  • 12
  • `ActiveSheet.Range("A1").Offset(0, Plot).Value = Plot` This line starts at `A1` and offsets by `Plot` columns. It would be better to use `Worksheets("Sheet1").Cells(1,Plot) = Plot`. `Cells` refers to a single cell by row and column number (so great for using in loops). Also better to refer to specific sheet rather than `ActiveSheet` as they may have changed next time you run the code. To refer to a range of cells using `Cells` you'd use something like `Worksheets("Sheet1").Range(Worksheets("Sheet1").Cells(1, 1), Worksheets("Sheet1").Cells(4, 5))` - A1:E4 (row 1, col 1 to row 4, col 5). – Darren Bartrup-Cook Aug 02 '17 at 08:37
  • I read that you shouldn't refer to specific sheet names as people often rename them etc breaking the macro? – Dekks Aug 02 '17 at 08:41
  • 1
    That can happen and can be a problem. You could refer to the sheets codename which the average user can't change (you can only do it in the VBE). When you look at your `VBAProject` where the modules and Excel Objects are displayed it will say something like `Sheet1(Sheet1)`. The name in the brackets is the name as displayed on the tab, the other name is the codename and is a direct reference to the sheet object so you could write the reference to A1:E4 as `Sheet1.Range(Sheet1.Cells(1, 1), Sheet1.Cells(4, 5))`. – Darren Bartrup-Cook Aug 02 '17 at 08:48
  • Or you could protect the workbook so the user can't change the name. – Darren Bartrup-Cook Aug 02 '17 at 08:50

2 Answers2

1

Select is inefficient. Instead of selecting offset cells, try referring to cell like this: Cells(row, column).Value, with this you can get or set the value of cell on the specified row and column (you can use Formula instead of Value and many others :) accordingly to your needs ). That should speed up your macro. Generally, in VBA we have Range type, which can represent a range in a sheet. Thus, you don't need select specific ranges (which slows down execution) in order to refer to them :)

Also, it's better to use Long instead of Integer, because all integers are converted to long in VBA anyway, refer to this post.

Michał Turczyn
  • 32,028
  • 14
  • 47
  • 69
  • I will play around with Cells(x,x).Value. I did consider whether for each row I would just turn each range into an array and write to that, but its a little beyond me right now, baby steps... :) Yes, it took a google of why I was getting a overflow error before realizing Integer had a limit. – Dekks Aug 02 '17 at 08:39
  • Using Cells instead of offsets, changed calculation time from 179 seconds for 1000 x 1000 times table, to 48 seconds, nearly three times faster. Amazing what a difference one little change makes. – Dekks Aug 02 '17 at 09:01
1

As already pointed out, .Select is not necessary and slow. What you want is to refer the Cells directly. Below i have an example without .Offset and .Select. 

Sub TimesTable()
Dim ws As Worksheet
Dim TimesTable As Long, Plot As Long
Dim Numbers As Long, RowNumbers As Long
Dim StartTime As Double, SecondsElapsed As Double
Dim Cell As Range

Set ws = ActiveSheet

'Turn off various things to speed up macro
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual

'Start Timer
StartTime = Timer
ws.Cells.ClearContents

'Ask for and read input into variable
TimesTable = 100

'Write Axis lines
With ws
    For Plot = 1 To TimesTable
        .Cells(Plot + 1, 1).Value = Plot
        .Cells(1, Plot + 1).Value = Plot
    Next

    For Each Cell In .Range(.Cells(2, 2), .Cells(TimesTable + 1, TimesTable + 1))
        Cell.Value = .Cells(Cell.Row, 1).Value * .Cells(1, Cell.Column).Value
    Next Cell
End With

'Turn on screen updating etc again
Application.ScreenUpdating = True
Application.Calculation = xlCalculationAutomatic
SecondsElapsed = Round(Timer - StartTime, 2)
'Display time taken to run macro
MsgBox "This code ran successfully in " & SecondsElapsed & " seconds", vbInformation
End Sub

I just tried it with TimesTable = 1000 and it saves around 20Secs. The main difference is, that i used a For Each-Loop instead of 2 For-Loops. The For Each goes trough every Cell in the Range where the multiplicated values should be. Also to makes things shorter you can use vars for Workbooks, Worksheets, etc., also a lot easier to modify afterwards.

Plagon
  • 2,689
  • 1
  • 11
  • 23