Code: Standards

Topic

New Reply

*** Edited by GeoffW. Chris: I’ve removed the “pre” tags because text doesn’t wrap- There’s a lot of scrolling “right” to do otherwise ***

1. Option Explicit
I always use Option Explicit; anything that keeps my code strict is good.
2. Sub cmd_RemoveAllGraphics()
I try to previx user commands (can be invoked from a toolbar button) with cmd_
3. ‘ Tested: By the calls shown below.
Because my ProcStrip utility now sorts all procedures in a template, I am embedding the test code as a commented block immediately preceding the END statement.
To run the test you have to drag the commented TEST outside the procedure, decomment it, and run it.
The tests are supposed to (a) give you a chance to see how you might call the procedure (2) give you a chance to single-step through and observe the process (3) provide a testbed for any future changes.
4. Function boolClearAllGraphics(doc As Document, strReplace As String) As Boolean
I use, I think, Reddick naming convention. I try to be consistent at any rate.
My current theory on procedures (subroutines and functions) is that (a) functions are better than subroutines because they return a value ( writing a procedure as a subroutine is an admission of failure on my part © because digital computers are essentially mondaic and dyadic beasts, functions should contain no more than two arguments; this promotes a trend towards small building blocks of reusable code. Each function composes two arguments into one value. (d) any procedure that the user isn’t allowed to see should be written with an argument so that it doesn’t show up in the Word97 Tools, Macro, Macros list (e) The function doesn’t exist that can’t be made to return a value, even if it is only a boolean success/failure.
5. Selection.Find.ClearFormatting
I make no bones about it. I use Tools, Macro, record to initiate many of my efforts. It does not produce efficient code, but it produces code efficiently.
6. .Text = “^g”
7. .Replacement.Text = strReplace
I usually try to anticipate parameters to low-level functions. In this case I have anticipated the .Replacement string (you could replace a graphic object with the string “Graphic”); my mind went blank with the .Text string.
8. ‘Sub TESTboolClearAllGraphics()
9. ‘MsgBox boolClearAllGraphics(ActiveDocument, “”)
10. ‘End Sub
Typical TEST procedure, capitalised letters TEST followed by the name of the procedure being tested. Minimal work. MsgBox lets me see the result from Word without labouring through the Debug window.
Some TEST procedures carry a comment on each line predicting the result; very useful for re-testing after a change in code. Indebted to Poole&Waite for this approach.
11. lngRemoveShapes = ActiveDocument.Shapes.Count + ActiveDocument.InlineShapes.Count
I’s rather increment a local counter in each loop, but I’d get yelled at as “inefficient”. A local counter gives me more control over the debugging/inspection process, which is where most of my life goes

Reply | Quote

Replies

Hi Chris,

Thanks for posting these notes and the RemoveAllGraphics code (I’m afraid the two threads are going to get separated now!).

The code looks really clean and straightforward; nothing important to comment on.
Just a few very minor comments/questions:

“Functions are better than subroutines” – I’ve gotten the same advice from a few veteran types now; the logic seems inarguable so maybe it’s time to break the old sub habit (have to admit the simple sub is easier to write).

Code via Recorder: no doubt it’s useful. There are instances though where the code it produces is quite inefficient and in some cases the objects/methods aren’t the best ones (for one example, the Recorder is addicted to the Selection object, while Ranges are often much more efficient).
In the current code, the only picky point that can be made would be that you can move the first two “Selection.Find” statements inside the “With Selection” part just below.
(You could also substitute “ActiveDocument.Content” (a range) for the “Selection” which the Recorder gave you.)
And similarly the “boolClearAllGraphics = Selection.Find etc. can be moved up inside the With/End With.

lngRemoveShapes: you’re returning a Long, why not declare intShape as a Long too, rather than an Integer?

“Increment a local counter in each loop”: I take this to mean that you’d rather have a separate counter variable for each loop, rather than reusing “inShape”? – I don’t know about the efficiency argument, but I thought it was good programming practice not to reuse a counter like this. The efficiency argument seems spurious; how much overhead does an additional long variable add?

As you say, debugging/inspection is going to go much easier if they are separate variables. Seems to me clarity of code should win if the performance difference is negligible.

Thanks again for the code,
Gary

Reply | Quote

>have to admit the simple sub is easier to write

How so? Anything a SUB can do, a FUNCTION can do. Except a FUNCTION can’t be used as a macro (“can’t be accessed through Word97’s Tools, Macro, Macros menu). And who cares a hoot about what the user wants, right (Very Big Grin)

It’s a lovely mental game to find a SUB that **can’t** return a value that might be of some use.

> Selection object, while Ranges are often much more efficient

I quite agree. Selections retarded my development. I try to use ranges now.
However for speed of determining if something-can-be-done, for me Recorder is unmatched. Trouble is, once I’ve established **A** solution, I tend to move on to the next problem, rather than going back to make the first solution slightly better,

My best example of this was Indexer, which originally worked on the active document. Then I made it work on a Range, then a Selection, then a string of text. The end result was a lovely set of about six functions, any one of which could be used to pluck “interesting words” out of anything at all. Ranges let me magnify the utility of my work. In this case I wrote a utility that could build TOC headings out of paragraphs of text, and apply an appropriate level of style (Heading 1, Heading 2 etc.)

> can move the first two “Selection.Find” statements inside the “With

Thank you. And I delete the “selection.find” part of each of the two lines. That’s more elegant/readable, right? Nothing to do with speed of execution. A legacy of my habit of recording code and then tweaking it.

> You could also substitute “ActiveDocument.Content” (a range) for the “Selection” which the

This too is a result of my recording rather than writing. As well it is yet another of my cheap’n’nasties that grows over a period until I figure it is time to go into a full-blown application.

> argument, but I thought it was good programming practice not to reuse a counter like this.
> As you say, debugging/inspection is going to go much easier if they are separate variables.
> Seems to me clarity of code should win if the performance difference is negligible.

I’m still arguing this particular coding practice with myself, but the debate should be over in a year or two. I think this is an exception to the exception to the rule, sort of.

I anyone but me had written the code, I’d have reacted as you did. Separate variables is clear, and who has been worrying about storage space since the last IBM1401 was unplugged?

I had visions of a series of maybe ten loops, each dealing with a particularly annoying type of graphic, and I thought if I used one common identifier for each loop counter it might show that it was a series of similar-purpose loops (“get all of THESE sorts of shapes”), rather than a series of functionally different loops.

Reply | Quote

Gary,

For other readers of the thread, it’s been moved here– the original is buried a bit deep.

Reply | Quote

Chris,
I think the issues of SUB vs FUNCTION, and the building of procedures in general are worthy of further discussion, and can help us all become better programmers. (Note: I will try to use “Procedure” to refer to either a sub or function, but the terms “sub” or “function” when I mean one of them explicitly).

You have promoted two concepts in various threads, that I wish to respond to.

A Sub should never be used.
First, and I think you have pointed this out, a SUB is necessary when programming in the VBA environment, if you wish to expose your macros to the user.

My personal opinion is to follow the convention that a Function returns a value, whereas a Sub does not. Most standard coding that you will read uses this convention. When you start to program subs as functions, returning essentially useless values from them, you make the calling procedure harder to read; you expose your code to more programming errors, and you lessen the portability of the sub that is now a function procedure. (for instance, you run the risk of moving it’s error checking into the calling procedure, as opposed to raising an error in the sub which is usually better). You stated that you challenge people to write a sub that cannot return a useful value, but I would say that in many instances, a sub (written as a function) can only return a value of whether it succeeded or failed, and in most of those cases, efficient error code in the sub is more appropriate than in the calling procedure. The exception sometimes being when the calling procedure must fail if the sub fails, but again, raising an error may be more approprite for handling this, since at best, a procedure failing because a sub function failed will never tell you why the sub function failed, only that it did, whereas a raised error can tell you that information.

A Function should only take in two values, and then return one value.
I strongly disagree with this statement, and feel it limits and unnecessarily complicates code. My definition of a function (or a sub for that matter), is it should perform one specific task, and it should perform it well. To arbitrarily limit these values to only two values in does not in my opinion constitute good coding practice, rather it limits the ability to code good, and can hinder instead of enhance the portability of your code. Example: (pseudo – coded)

Function CompTrapezoidArea(aSide, bSide, Height)
     CompTrapezoidArea = 1/2(aSide + bSide) * Height
End Function

Breaking the above into two functions would serve no purpose other than to unnecessarily involve an intermediate step.

I’ve enjoyed your code samples, and look forward to seeing more of them. I do think it is helpful for all to hear differing viewpoints on various coding styles and develop styles that work well for themselves, and the people who use there code.

Reply | Quote

James, thank you for pursuing the post/response. I was, honestly delighted to be taken to task. I have suffered over the years of programming in isolation. My views happen to be current, but they are the result of changes over the years, and the change is good. I am digesting your comments; in the meantime (grin!):

>a sub (written as a function) can only return a value of whether it succeeded or failed,

I used to think this, and I’ll accept that there may be such acses. I have been finding that I can often return a value other than success/fail. For example, the strPP function (which writes data to the INI file) can return a previous value.

I had to ask myself what I would do if strPP failed. For this particular function to fail, it would have to be a catastrophic failure, such as the core VBA interpreter having problems, or a file acces sor similar. In such a case raising an error in strPP and terminating the application would make sense.

In the meantime, giving the option of letting the calling procedure log the old value and the new value would be of benefit in some applications.

I’ve avoided the ON ERROR business because I hate the GOTOs it involves, and I seem to need ON ERROR most of all when Word/VBA hasn’t given me a means of obtaining the error by controlled means.

>I strongly disagree with this statement, and feel it limits

How strongly? I did say “should”, but I might have better worded that as “I strive towards this goal”. Your example function is a perfect example of why my guding statement should be bypassed.

> and look forward to seeing more of them.

Thank you. These are the awkward-gosling stages. One has to start somewhere. I think that if I am permitted to post one per day that the light will soon shine. The INI functions and my little library of string-manipulators have become second nature to me.

Reply | Quote

>>A Function should only take in two values, and then return one value.

> … a function … should perform one specific task, and it should perform it well. To arbitrarily limit these values to only two values in does not in my opinion constitute good coding practice

OK.

I mellowed out a month ago, but only got around to replying now. You’re right. It’s really NOT right of me to say “Thou shalt use only two arguments”. You guessed right if you guessed that three days after my original post I discovered a case where I COULDN’T restrict myself to two arguments!!

I’m now at the point of saying “Strive to write procedures with as few arguments as possible”, with a view to developing procedures that have good chances of becoming useful building blocks in later life.

I’m looking at SGerber’s latest contribution (incerementing numbers in mixed strings) and thinking that out of all of that there have to come several small utility functions that decompose a string into component parts, loop along sub-strings, increment, and re-compose etc. His original fucntion works for his app, and that’s often what I end up doing when I am impatient to see the app run. Later I realise that if I’d broken it all down into house-brick size, I could (i) have written a better driving procedure and (ii) made use of the building blocks elsewhere.

It does NOT go without saying that I’ve been given much reason for thought these past two months on account of your reply. In delaying this long in replying I’ve done your message an injustice. Thanks for making me think a little better!

Reply | Quote

In addition to the original suggestions, I’d also suggest the following…

Try to program using dispatch mode. One sub or function does the dispatching. It calls nearly all the other subs or functions. Try to avoid one routine that calls another routine and that routine calls another routine and so on and so on.

When commenting your code, put less on what your code is doing and more on WHY you’re doing it.

Stick to a variable naming convention.

If I think of any more, I’ll post ’em.

Mike

Reply | Quote

>Stick to a variable naming convention.

I’m basing my naming on the Reddick(?) naming convention. What is your preference? My Adobe PDF file is “AppC — Reddick VBA Naming Conventions.pdf”, but I can’t find a reference on the web.

On request I’ll u/l a copy of the file to my web site.

Reply | Quote

I’m not sure of the name, but it’s at the following link
http://msdn.microsoft.com/library/devprods…conventions.htm

It uses the 3 character prefix for variables and controls.(strFirstName, intAge, …)

The other variable naming convention I’ve seen uses a single character prefix.(sFirstName, iAge, …) I’m not sure, but I think this one is called Hungarian.

I’d be interested in seeing the one you use.

I guess the real point is to make sure your team sticks to one convention. It’s not good to have that rouge programmer using his own style while his/her peers stick to something else.

Mike

Reply | Quote

> I guess the real point is to make sure your team sticks to one convention. It’s not good to have

Quite so. I hold that I believe in “conventions” above any particular convention. As a gypsy, I have to subscribe to whatever convention the shop has adopted.

Reply | Quote

Microsoft has promoted that naming convention for VB 6, and it isn’t quite Reddick or anything else, but it’s close. I find it extremely annoying because of its differences, which seem entirely arbitrary, like using vnt for variant instead of the more familiar var.

Hungarian notation does NOT use single character tags, it uses 3-letter tags more consistently than Reddick does. Hungarian notation is so-called because it was developed originally by Stan Leszynski and co-published with Reddick, originally as the Leszynski-Reddick Naming Convention. Over time the two diverged, and I prefer Stan’s version which is available here among other sites.

Reply | Quote

Charlotte,

Thanks for posting that link – now I’ve got both bookmarked.

The development team I work with enforces naming conventions that seem a little ‘unconventional’: the three-letter prefixes are used for forms objects (lbl, txt, chk, cmd etc.), but for variable data types, a single letter is used (s = string; n = all number data types; b = boolean, v = variant).

Are you familiar with this type of naming convention; and if so would you happen to know anything about its pedigree?

Thanks,
Gary

Reply | Quote

No, I’m not familiar with it, but it sounds like something that carried over from older languages where every character took up precious memory and they were running with 56K of RAM. It seems to go with the Dim i as Integer school of programming. And you also see a lot of use of f as an indicator for booleans, f standing for “flag”, you see.

Until Leszynski and Reddick published their naming conventions, I had never seen a methodical approach to this; but I’ve sure seen a lot of code that uses things like D1, D2, D3, R1, R2, R3, etc., with no hint of what the darn things are supposed to contain except their declared data type. Frankly, I prefer even that to type indicators in variable names. I can never remember what they mean–I remember $ for string, but I can’t remember whether % is an integer or a double, and I don’t want to have to.

Reply | Quote

Thanks for the clarification.

Reply | Quote

You can get the most up to date version of the Reddick naming conventions online here.

Reply | Quote

Thanks!

Reply | Quote

Further to the discussion of Naming Conventions, what are (any of) you using as a prefix for cell elements of a Word table?

I find nothing in the articles of the Reddick Naming Convention.

Reply | Quote

Thanks. The prefix for Spin Buttons got here just in time (wanders off to edit “sb” to “spn” ….)

Reply | Quote

You’re welcome.

I’ve been writing Functions for everything EXCEPT user macros accessible via the Tools, Macro, macros (which is equivalent to saying “can be dragged onto a toolbar).

I had been prefacing those user SUBs with “cmd”, but I’ve learned that that’s a bad habit, and useless too, as it turns out.

I think I’d started the cmd_ thing when I was putting user macros on command buttons floating above the document.

Then Forum members taught me about toolbar menus.

Reply | Quote