other that the above two lines of code, what can i use to speed up my macro?Code:'... speed up macro
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Printable View
other that the above two lines of code, what can i use to speed up my macro?Code:'... speed up macro
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual
Many things, but it is not hard and fast. You canQuote:
Originally Posted by crmpicco
- remove all selections in your code, and reference via the object,
- look at all your loops and see if it could be done another way
- etc.
and then it is down to the design, seeing if the design is best suited to the task, or whether another approach might be more optimal (for instance, maybe use more Excel built-in functionality.
how would i go about that?Quote:
Originally Posted by xld
for e.g. i am selecting the cell below:
is that what you mean? Change this to object reference? How would i go about that?Code:Range(sRange).Select
Where you see code likeQuote:
Originally Posted by crmpicco
[vba]
Range("C2").Select
Selection.FormulaR1C1 = "=VLOOKUP(RC[-1],ref!C[-2]:C[-1],2,FALSE)"
[/vba]
it is better written, and faster, as
[vba]
Range("C2").FormulaR1C1 = "=VLOOKUP(RC[-1],ref!C[-2]:C[-1],2,FALSE)"
[/vba]
and so on and so on
is .Select a resource-hog?
thanks for that, i have removed the .select from my code, anything else to look out for? excluding looping structures
Here is an extract from some work in progress
A frequently emphasised point made to less experienced programmers when their code is 'exposed' to public scrutiny is that it is carrying a ton of overheads in the form of redundant code. It is especially pointed out that it is rarely necessary to select any object to act upon that object. The reason that their code suffers this problem is obvious. One of the easiest ways to learn about programming with VBA and the Excel object is to use the Macro Recorder. It's a very useful facility, but it has to cater for so many possible combinations that it takes 'the path of least resistance' and creates generic code which is heavily redundant. writeTitle(4,5);Just use the macro recorder yourself and you will see code that is peppered with '... .Select or ... .Activate and/or Selection. ...'. This is because the macro recorder literally records evey step taken whilst it is turned on, every scroll up or down the page, every cell activated, every sheet change. This gives rise to unnecessary code.
Supefluous Code
Whilst the Macro Recorder can be very useful, it is better to remove the redundant code, partly to make the code more efficient, partly to make it more readable, and partly to help you understand how to write such code from scratch.
This section covers a number of the most common redundancies introduced by the Macro Recorder, and how to address them.
The first step to take is to remove all of the necessary Selects. Look for pairs (or more) of lines that start with the .Select or .Activate method, followed by use of the Selection or ACtiveSheet objects. For instance, the following code simply selects cell A1, then copies it to cell B2.
[VBA]Range("A1").Select
Selection.Copy
Range("B2").Select
ActivSheet.Paste
[\vba] Removing the obviously redundant code, this could be written as [vba]Range("A1").Copy
Range("B2").Paste[/VBA]
The first '.Select ... Selection,' pair can simply be removed and join the statements. The net effect is the same, but the selecting does not happen, making the code more efficient and more flexible (more later). The same can be done with the second '.Select ... ActiveSheet.' pairing. 4 lines reduced to 2, and more readable. This shows another commonly used feature of recorded code, copying and pasting in two steps, when it can be simply done in one. This is achieved by including the 'Destination' argument to the 'Copy' method instead of using the 'Paste' nethod on the Range object. This would then be written as
[VBA] Range("A1").Copy Destination:=Range("B2")[/VBA]
This seems to achieve what we intended, and we have reduced four lines of code down to a single line, couldn't we better could it? Well pootentially yes it could. If you look at the code, we are copying cell A1 and replacing B2 with it. If we are only wishing to copy the cell value, this could more easily expressed as
[VBA]Range("B2").Value = Range("A1").Value[/VBA]
This could also be written as
[vba]Range("B2") = Range("A1")
[/vba]
but I would discourage this practice of using default properties - see below for comments on coding style. Note that this method does not work if you are copying some particular attributes of the cell, such as formula or formats. In this case, you would have to use 'PasteSpecial' rather than 'Paste, so here you still need to 'Copy' and 'Paste' and do it in single steps, such as
[vba]Range("A1").Copy
Range("C1").PasteSpecial Paste:=xlFormulas, _
Operation:=xlNone, _
SkipBlanks:=False, _
Transpose:=False
[/vba]
Note in the previous example how I have broken those statements down over separate lines and indented. See below for my views on coding style. Another common example of superfluous code created by the macro recorder is when it sets many attrributes as well as the intended one. As an example, consider this macro that was recorded when simply setting a cell's horizontal alignment to centre, i.e. just clicking the button on the Format toolbar.
[vba]Sub Macro1()
'
' Macro1 Macro
' Macro recorded 21/11/2002 by Bob Phillips
'
'
With Selection
.HorizontalAlignment = xlCenter
.VerticalAlignment = xlBottom
.WrapText = False
.Orientation = 0
.AddIndent = False
.ShrinkToFit = False
.MergeCells = False
End With
End Sub
[/vba]
Apart from the obviously redundant comments at the head, there is much other redundant code. All we did was to set the horizontal alignment to centre, but you will see that it sets many other properties. If you look at the format dialog box (menu Format>Cells - Alignment tab) there you will see a number of options, all of those set by the macro. It would seem that record macro uses that dialog, or the code invoked by that dialog, somehow beneath the covers. We should only keep those property settings that we actually want to change, as most of the others will just be setting it to its existing value. By stripping out all of the unnecessary property settings, we are left with
[vba]Sub Macro1()
With Selection
.HorizontalAlignment = xlCenter
End With
End Sub
[/vba]
This makes the 'With' statement redundant, so we can further reduce it to
[vba]Sub Macro1()
Selection.HorizontalAlignment = xlCenter
End Sub
[/vba]
Yes, it requires a screen re-paint.Quote:
Originally Posted by crmpicco
that's time-consuming, needs to undesrtand the code, that's consuktancy.Quote:
Originally Posted by crmpicco
anything stick out to you in this loop?Code:'... loop up the rows to check if the cell contains a valid date
'... if it does then convert the dates to DB format
For d = tempCol To 1 Step -1
sPeriodRange = getColumnLetter(d) & tempRow
If hasDate(Range(sPeriodRange).Text) Then
sFarePeriod = stripPeriods(Range(sPeriodRange).Text)
sFarePeriod = convertPeriodDates3(sFarePeriod)
End If
Next d
Not really, but you don't need the HasDate function, and I have no idea how efficient StripPeriods and ConvertDates are, but you do seem to do unnecessary stuff (suh as your own date checker, and converting columns and rows to a string), but my guess would be that they are at least partially superfluousQuote:
Originally Posted by crmpicco
[VBA] For d = tempCol To 1 Step -1
If IsDate(Cells(temprow, d).Text) Then
sFarePeriod = stripPeriods(Cells(temprow, d).Text)
sFarePeriod = convertPeriodDates3(sFarePeriod)
End If
Next d
[/VBA]