PDA

View Full Version : Optimisation tips



glencoe
02-19-2014, 05:58 AM
Hi guys,

I thought I would start this "optimisation tips" thread, to gather tips from the best. I have already learnt through previous posts a few "rules of thumb" and other "best programming practices", but I wanted to go further here, by gathering whatever you consider as the best practices in VBA.

So, here is what I already know...

1) Variable declaration rules:
- Add a prefix related to the variable's type (str for strings, o for Ranges or Objects, etc.)
- Use names that explain what variables are about (some autoexplanatory name is the best)
- Replace all Integers by Longs. The reason was explained to me by fumei in a previous post:

Integers are no longer used at all by VBA. You can declare them, but the parser converts them to Long on its initial parse pass.
- Destroy Set object instances before exiting a routine (Set object = Nothing). Again, fumei gave more detail about it:

In theory, when you exit a routine with a Set object (for example Set oHF = a headerfooter object), that object is destroyed. But there is still persistent issues that pop up with application objects - i.e. "extra" instances of Word.


2) Mind the scope of your variables. Nothing is more stupid to think a variable is global when it is local. That leads to many mistakes. Avoid using similar names for global and local variables. You only look for trouble.
Now, I am not sure (if someone can tell) if there is any advantage in declaring a local variable and use it as an argument of a function rather than declaring it as global.


3) Use Range rather than Selection
Though this is another world of programming, Range is definitely more efficient than Selection, at all levels: number of required lines, speed, versatility


4) Whenever possible, use an Iif statement instead of an If Else End loop. Unless I am mistaken, this is perfect for one line If loops, such as:


If A>B then
A=B
else
A=C
end if

A more efficient way would be:

A= Iif(A>B,B,C)


5) Now, I wonder if writing an If statement on one line is more efficient than writing it on 3 lines?

If A=B then
strText = "blabla"
End if

or


If A = B then strText = "blabla"

I like better the 3-line code, as it looks nicer (and more obvious on the screen), but if the 1-line programming is more efficient, I would go for it...


6) Any other tip or suggestion?

Please feel free to comment or add your tips, whatever they are!

Frosty
02-19-2014, 01:59 PM
You should always use local variables where possible. The concept of scope and lifetime is a broad one, as is optimization... and doing web searches for better resources may be better than just a forum (a number of us could probably write disseration-length posts to try and fully explain).

Variable prefixes -- it can be very helpful to add an additional prefix indicating scope. Using my prefixes, I would do something like this:
Public p_sTemp As String (this is a public string variable, available to any routine in the project, with a lifetime the same as the application, with some caveats (like use of 'End' -- which is generally not recommended)

Private m_sTemp As String (this is a private string variable, only accessable from procedures within the same module, but with the same lifetime as a public variable)

Dim sTemp As String (this is a local variable, available only to the procedure from within which it is declared, with a lifetime as long as the procedure is running).

I completely disagree with your preference of an IIF statement over an If...End If logic construct, for exactly the same reasons as I abhor single line If statements: DEBUGGING CODE.

"Optimized Code" is a pretty nebulous concept. Is it optimized for processing speed? Optimized for Debugging? Optimized for readability? Those are different masters, and you know the thing about serving multiple masters... you can't.

When given a choice, I believe you always 'optimize' code for readability first. That means following a few simple rules:
1. prefix your variables appropriately, with a scope appropriate to the variables usage. Having multiple Dim sTemp As String in 4 different procedures is FAR PREFERABLE than having a single Public Dim p_sTemp As String which is used in 4 different procedures. Why? Because you never run into some wierd bug which YOU have created because of a scope issue (i.e., one procedure setting your public string to a value, and another procedure getting that value when it is entirely inappropriate for that procedure).

2. Name your variables appropiately. "sTemp" as a variable name is a horrible name-- it has no meaning, except that it is a "temporary string variable" -- yeah, so what? They're all "temporary" variables -- it's just a question of whether the variable gets reset when you stop running the procedure or when you unplug your computer. So reserve variable names like sTemp for a truly temporary use - like being used as a holder for multiple string manipulation actions, so that you can see in your Locals window whether each step in the string manipulation is correct. Appropriate variable names becomes self-documenting.

3. Comment your code extensively. When appropriate variable names aren't enough (and they rarely are), comment your code so you don't have to run the procedure to see what it's supposed to do. Put comments at the top of any routine describing what it does and should do. If your comments describing what the routine should do are extensive, that's a good reason to break that routine into several routines. Also -- if you code well, you're not tweaking the code daily -- you're revisiting it months later when you've forgotten everything about it. So write your comments for someone else, even if that someone else is your "future self" who doesn't remember squate about what you were doing.

4. Organize your code for readability and the ability to easily test. Entire routines don't break. Portions of those routines break because you haven't thought of something. Break your routines into their constitutent parts, so that you can more easily test those parts which aren't working and skip all the parts that do. And use multi-line statements whenever possible. You can't put a break point within a logic chunk when that logic chunk is a single line (like IIF or a single-line IF statement). So you have to put your breakpoint on the line itself, even if you only wanted to test what it does when, for example, the value is False (or True).

Frosty
02-19-2014, 02:05 PM
And lastly-- there are no real rules (except the one about commenting -- non-commenting coders are about the only thing that drive me nuts in programming... well, that and bugs in syntax rather than logic).

Not commenting your code is only attributable to laziness, slopiness and/or thinking you're smarter than everyone else. None of those attributes actually help anyone (even yourself) write better code. They may help in some kind of therapy/issue area, but that's pretty far from my area of expertise.

Some people can instantly decode a complex nested string manipulation statement... so they would 100% disagree with having more lines of code in order to debug. And I can't disagree with them -- as long as they provide a comment that says something like
'convert "XYZ123" into "1X2Y3Z"
So that someone else can come along and at least understand what the before and after is supposed to look like. I'd prefer that it stepped through the process in multiple lines of code, but the only real rule is that my Future Self not have to try and wade through a multi-nested operation on a single line with zero guide-posts other than stepping through the code.

glencoe
02-19-2014, 02:09 PM
Good points Frosty! :-)
I must agree indeed that optimising code depends on what you mean... You can't achieve readability and highest speed at the same time. But you can sometimes compromise I guess...
Any other suggestion?

Frosty
02-19-2014, 04:55 PM
Honestly, if you're really looking for speed, VBA isn't where you'll achieve it. Compiled managed code into a com addin or .NET or a .dll will result in significantly faster processing. But at the expense of easier debugging and real time access to code in a global VBA addin template. Since this is VBA, I believe the focus should be on optimization for speed only when you're seeing real noticeable issues with performance.

SamT
02-19-2014, 07:50 PM
Always optimize VBA code for reading comprehension. Even when optimizing for speed.

One liners have their place. That is when a common and simple task is being done.
If Target.Count >1 Then Exit Sub
LastRow = Cells(Rows.Count, 1).End(xlUp).Row

Sometimes a bulleted list type of style is easier to comprehend
Select Case X
Case 1: Y = "Apple"
Case 2: Y = "Ballet"
Case 3: Y = "Charles"

The only three single letter variables you should ever use are i, j, and k. If you nest loops deeper than three, use meaningful names for the counters.

Type prefixes have little use in VBA, because most VBA code is less than two screens long. Often the variable name is more helpful since InteliSense came along.
strRange As String
'vs
RangeName As String

Optimizing for speed and debugging will happen automatically when you really optimize for reading comprehension, simply because you will really understand your code and can't help but see how to make it faster and leaner. Seriously. I spend more time editing the code to make it more readable than I do on the initial code. Invariably, the algorithm itself turns out better and faster.

Frosty
02-19-2014, 08:14 PM
Note that SamT and I can reasonably disagree about some things (I regularly use x and y as single letter variables too, although I agree with the premise, and generally only use single letter variable names for integer/long types), and I would even strongly disagree with not prefixing variables, as well as sticking to my guns on not using single line statements-- but because of the way I debug (I make heavy use of both break points and the locals and watch windows, as well as the immediate pane), rather than because of disagreeing about readability. Because of this, I hate single line case statements, because then I can't put a break point at a spot where I'd only stop execution when a particular logic expression is met (i.e., when SamT's Case 2 is met).

But that said, two experienced coders with (I'm guessing) pretty different styles of coding, agree on making the code readable and self-documenting.

You can't get a better tip than that, as a guiding principle. The rest of the details will come with experience.

glencoe
02-20-2014, 04:13 AM
Thanks for sharing your thoughts!
What we have here is interesting: 2 schools of coding... I guess that if both schools have valid and valuable arguments to make their point, they also tend to show rather personal tastes...

What I still wonder is whether a one-line If loop is processed faster than the equivalent 3-line loop? I KNOW that we can't really talk about a major improvement, like using Range over Selection! My experience on this talks for itself... As for a 1-line If, when it is simple enough, and that debugging is not required anymore, it may be interesting to use. But I do agree with Frosty that it is bad for readability and debugging! I actually don't like that kind of coding, but again, that's quite a personal opinion.

I don't want to start a heavy discussion on personal taste. If you have any other argument to bring, I'd like to hear it. But please don't start a "battle" between 2 schools! ;)

SamT
02-20-2014, 06:22 AM
One and three line Ifs are processed identically

IF condition Then action [End statement]

The Condition will be evaluated in both

If true, the action will be taken in both

The end statement is present in both, but in the one liner, the end of line is the end statement.

However, if you want to watch both Condition and Action, you must use a three liner because you cant put a Break Point in the middle of a line.



At last count, there were 3902.836 correct coding styles. They all had one thing in common; readability!

glencoe
02-20-2014, 07:46 AM
Perfect! Let's stick to readability then! :)

gmaxey
02-20-2014, 08:30 AM
glencoe,

I've visited your namesake region of Scotland many times. It is a stunningly beautiful place.

To some this is readable:
fcnLineList = Replace(Replace(strIn, "|", ", ", 1, Len(strIn) - Len(Replace(strIn, "|", "")) - 1), "|", " and ")

To me it is a riddle wrapped in a mystery inside and enigma. Just kidding, it is one answer to question which I asked about some of my own clunky code. I think you will agree that without commenting, it may be hard to look out in a month, six months or six years and readily see what is going on.

As for Range vs Selection, just don't be over zealous. There are a few instances where .Selection is required.

If you want to use the predefined bookmarks such as \Page or \HeadingLevel. These are always defined relative to the Selection.

Selection.Bookmarks("\Page").Select

If you want to move up or down one line (as opposed to one paragraph) or want to identify the current line of text.

Selection.MoveDown wdLine, 1

If you want to use some items returned by the Information property.

MsgBox Selection.Information(wdActiveEndPageNumber)

If you want to work with a column in a table. This is because a column doesn't have a range property.

'Shade the 3rd column of the selected table
With Selection
.Tables(1).Column(3).Select
.Shading.BackgroudPaternColor = wdColorRed
End With

I also use Selection.InsertFile because if you use Range.InsertFile, the range ends up at the beginning of what you've just inserted, instead of at the end, which isn't ideal, and Selection.InsertFile is a little faster.

And then there are what I call the anomalies. Things that seem should works, but for some reason just don't. For example say you want to remove all the trailing spaces between the end of a footnote and the final paragraph mark.


Sub ScratchMacro()
'A basic Word macro coded by Greg Maxey
Dim oRng As Word.Range
Dim lngIndex As Long
Set oRng = ActiveDocument.StoryRanges(wdFootnotesStory)
Do While oRng.Characters.Last.Previous = " "
lngIndex = lngIndex + 1
oRng.Characters.Last.Previous.Delete
If lngIndex > 100 Then
MsgBox "A circular loop condition exists and the last space is still there."
Exit Sub
End If
Loop
End Sub

Sub ScratchMacroII()
'A basic Word macro coded by Greg Maxey
Dim oRng As Word.Range
Set oRng = ActiveDocument.StoryRanges(wdFootnotesStory)
Do While oRng.Characters.Last.Previous = " "
oRng.Characters.Last.Previous.Select
Selection.Collapse wdCollapseStart
Selection.Delete
Loop
End Sub

Frosty
02-20-2014, 08:40 AM
I honestly don't recall if a single line if statement is technically processed at a different speed than a 3 line if statement or an IIF statement. It might be in some cases, but probably not the way you'd think.

Logically, they all mean the same thing, of course. And I can definitively tell you that "less lines of code" has zero relationship to actual processing speed when you're talking about just a different way of articulating the same logical statement. BUT, there can be differences in processing a select case vs an If...ElseIf...EndIf. As well as a hundred other differences (for each loops are always faster than for loops, and while/do loops can be different.

However, all that said, I think the general rule of thumb is that optimization for speed is a silly endeavor with VBA, because there are so many factors at play to determine processing speed-- that this falls into the realm of theory rather than practice. Obviously never declaring data types has some impact on speed (use of all variants must have some impact on memory usage, as opposed to appropriately typed longs and strings), but in practice you probably don't notice it.

No one is using VBA to do hundreds of thousands of transactions, such that a .01 second improvement in speed actually has a noticeable impact. So learning about ranges and collections will serve far better in actual speedy routines (i.e., working with the object model) than a theoretical discussion on how to code for fastest code.

So, in short-- even of a one line if statement DID process faster, you still wouldn't want to use it if it made your code less readable.

SamT
02-20-2014, 08:42 AM
Here is a little 13 line, (in original form,) function I picked up online and wanted in my Function library. The only edits I made to the OP's actual code are that in the Swap Numeric and Alpha section, he used the Mid function for both sides and I thought that it was more readable using the Right and Left functions. I also replaced all calls to the Len function with references to StrLen.

I immediately rewrote the code in my preferred style, but I haven't used it yet. The four-mark comments act as section headers and are also the actual algorithm of the code. I write them, (the alorithms,) first when writing original code. All other comments show the reason why I chose to use a particular item or procedure to future maintainers

I show the code in an image because VBAX imposes its own style, (especially white-space,) on any code inside Code Tags. You see different colors and Tab spaces than usual because I customized my VBA Editor using the VBA Menu>>Tools>>Options>>Editor Format.

As you can see, it still needs a Boolean helper function, called in the Pre-Process Tests section, to handle very mixed strings, and the Error in Usage section needs completion.

When understanding this code:
First read the Four-Mark comments and you will know what the code is doing.
Then it is easy to ignore the pale green comments while reading the actual code to see how it does it.
Blue words indicate Decision procedures and Reddish show the operation.

glencoe
02-20-2014, 08:56 AM
OK. As for speed, I got it. The best rule of thumb is to have a readable code. It won't do much to reduce If loops on 1 line, apart from making the code less readable.

Otherwise, gmaxey, I certainly agree that Selection is useful (otherwise it wouldn't exist in VBA), but as I learned a brand new world (to me) - i.e. Range - I thought I'd better try my best to use it over Selection!

Bascially, commenting is definitely a highly recommended practice. As for readability. Then, of course, each one of us has his own coding style...

Frosty
02-20-2014, 09:03 AM
Ah, these darn excel programmers and their confusing range objects. Glencoe-- that's an excel.range which is an entirely different animal than a word.range.

And that said, I would quibble with that function passing in a range object (excel or word) and write it so it simply accepted a string argument, and returned a modified string with letters first and numbers last. And it would error trap so that if there were any errors in the process, it would return either a blank string or the original string.

But it all quibbles aside-- the commenting is excellent. So even. Though my word VBA mind says ".Count off a range object??" -- I still know what the function is supposed to do.

Frosty
02-20-2014, 09:09 AM
And selection has its place. Most obviously: where is my cursor? And there will be times when it's simply not worth avoiding the selection object because you need to go through so many hoops to use a range. Or it's impossible to get the same info (the range of a selected column in a word table is entirely different than the selection).

But I think you should always start with .range instead. Greg has great experience, and that's an interesting difference between .InsertFile off a range vs a selection. But there are other ways to get around that (inserting a bookmark before .insertfile, or skipping the use if .InsertFile entirely and using building blocks/auto text instead). However-- no generic right answer exists. It's a judgment call based on experience and what you're currently coding.

glencoe
02-20-2014, 09:16 AM
I certainly started a topic that is far beyond my knowledge, even if my aim was to gain more tips to improve my coding abilities.
The good thing is, we can share experience on this forum, so that everyone can improve and learn (or teach others)! :)

SamT
02-20-2014, 03:53 PM
Heh! This darn Excel programmer can't even do a Word Mail Merge without help, which is how I came to even notice this thread. But, yeah, in Excel the Range is everything.

fumei
02-20-2014, 06:53 PM
Ok, been away and now will give MY two cents.

I agree with Frosty.

fumei
02-20-2014, 07:16 PM
.....nah, I can't keep my mouth shut.

I also hate single lines IF statements. The reason the semi-colon was accepted is a holdover from COBOL and the like. AND IMO it makes debugging harder as it is simply too darn easy to miss some logic. I never use them.

Frosty's comments on variables are excellent and for your own code, strongly recommended. True, I am quite sloppy when posting here, but for my own stuff I am more disciplined.
But definitely use locals when that is all you need. There is no point in using globals when you do not need globals. This is where understanding scope comes in.

The speed versus readability is a no brainer. Readability rules in VBA. There are SOME speed issues, but as previously stated in reality they are quite minor in modern computers. As for the 1 line versus 3 line IF, you have to remember that the ONLY difference (beside the serious readability issue) lies in the parser, NOT the compiler. Both execute exactly the same when the code is compiled for execution. As for IIF, beeech yuck. There may be the occasional use, but really...bleech. Learn to use Select Case properly. It will save you much work.

So learning about ranges and collections will serve far better in actual speedy routines (i.e., working with the object model) than a theoretical discussion on how to code for fastest code.
The above quote from Frosty really sums it up. Collections, Ranges, and the use of objects - THAT is what will make better code. That and readability.

Oh and Greg's comment re: Selection and Range. Yup. There are definitely instances where Selection is the route, but over all, for 96.413% of the time Range is better.

Oh and DO comment!


fcnLineList = Replace(Replace(strIn, "|", ", ", 1, Len(strIn) - Len(Replace(strIn, "|", "")) - 1), "|", " and ")

LOL! or maybe ROFL. Yeah.....

glencoe
02-20-2014, 07:26 PM
fcnLineList = Replace(Replace(strIn, "|", ", ", 1, Len(strIn) - Len(Replace(strIn, "|", "")) - 1), "|", " and ")

That reminds me of mathematical equations... It's not that complex, after all, once you decompile it! I am just too lazy to do it now... lolll

Well, I think every comment leads to the same suggestions, after all. Thanks guys!

Frosty
02-20-2014, 07:54 PM
Or, every suggestion leads to the same thing, after all: Comment.

:-)

glencoe
02-21-2014, 05:11 AM
lolll