# Which Is Better Coding?



## peterpre (Oct 29, 2011)

Which is better coding - A) or B)?


A)
Sub Rectangle1_Click()
For x = 1 To 1000
Range("A1") = x
Next
End Sub

B)
Option Explicit
Sub Rectangle1_Click()

'Procedure to do a count up to 1000 and to show the value in cell A1
'Original version: October 29 2011; Author: PeterCar

Dim iLoop As Integer
Dim iMax_Count As Integer

iMax_Count = 1000

For iLoop = 1 To iMax_Count
Range("A1") = iLoop
Next

End Sub

I would say that A) is better, because B) is lazy.


----------



## Norie (Oct 29, 2011)

Eh, surely you mean B) is better and A) is lazy?


----------



## peterpre (Oct 29, 2011)

Norie said:


> Eh, surely you mean B) is better and A) is lazy?


 
No - I mean A is better. 

If the task to be done is really simple, then it is inefficient to type extra code. 

The "conventional wisdom" is to say something like "You should always use "Options Explicit" and have a systematic naming convention".

However, in simple macros, such as the one above, it is actually easier for subsequent users to read A) rather than B), and, as I say, B) requires extra typing - a waste of time.

Now the response might be "I like to keep to a disciplined approach in all cases". But this is lazy. The skill is to determine when a macro is simple enough to not require "accepted best practices". Of couse, I gave a very simple example . But there is definitely a line that needs to be determined.

As I say, the lazy intellectual approach is to always "stick to the rules". The better coding approach is to decide what approach is required for each specific situation.


----------



## Norie (Oct 29, 2011)

If it was something as simple as the example, or a one-off, perhaps it would be OK to use A).

Really don't think so for something more complicated that might be used and viewed by others after you long gone.

Mind you I'd probably do it like this.

```
Range("A1") = 1000
```
Kind of cuts out the other (unneeded) 999 iterations of the loop.


----------



## peterpre (Oct 29, 2011)

Norie said:


> If it was something as simple as the example, or a one-off, perhaps it would be OK to use A).
> 
> Really don't think so for something more complicated that might be used and viewed by others after you long gone.
> 
> ...


 
... although not if the user enjoys seeing the brief flicker of sequential numbers!

But it is a serious question - I think there is an awful lot of hours wasted across the world every day by people "over engineering" and "over commenting" code. There needs to be a balance, not a "one size fits all" orthodoxy.


----------



## Norie (Oct 29, 2011)

All you need to do to add Option Explicit to each module is goto Tools>Options...

There's also tools out there, eg MZ-Tools, that will automatically create code stubs for you which include the type of header info you included in B).

A lot of people will argue, including myself, that taking the time to do the sort of thing in B) actually saves time in the long run.

The code's easier to debug, easier to understand and follow, easier to alter, easier to re-use etc.

Anyway, I don't think there is a 'one size fits all' orthodoxy - a lot of coding styles are down to personal choice.


----------



## peterpre (Oct 29, 2011)

Norie said:


> All you need to do to add Option Explicit to each module is goto Tools>Options...
> 
> There's also tools out there, eg MZ-Tools, that will automatically create code stubs for you which include the type of header info you included in B).
> 
> ...


 
I agree about Option Explicit, and that there are good tools out there. 

By "one size fits all" I mean, for example the idea that "you should always use a systematic naming convention", and was not referring to any particular naming convention.

I think the issue is essentially an optimization problem. Let's think about the US as whole. We probably have about, say, 1 billion hours per year being spent by the various programmers creating code - a significant investment of time.

So how do we minimize the total time spent to achieve the necessary output - which is working programs.

If all code, whether simple or complex, was undocumented and used uninformative variable names, then the hours spent creating the code would be lower, but the hours spent updating or correcting that code would be higher - because people would be spending ages trying to understand what the code is doing.

Conversely,... well, you get it.

So the question is: have we got the right balance? 

I am saying we have gone too far in one direction, whereas, I think you are saying we currently have it right.

Difficult issues - but it really does deal with thousands of hours of people's time,


----------



## Norie (Oct 29, 2011)

A) Slightly quicker to write, lot longer to fix.

B) Slightly longer to write, lot quicker to fix


----------



## peterpre (Oct 29, 2011)

Norie said:


> A) Slightly quicker to write, lot longer to fix.
> 
> B) Slightly longer to write, lot quicker to fix


 
For Simple Requirements:
A) Quicker to write, same time or slightly quicker to fix
B) Longer to write, same time or slightly slower to fix

For Complex Requirements:
A) Quicker to write, longer to fix
B) Longer to write, quicker to fix

So question is: where is the line between simple and complex?


----------



## peterpre (Oct 29, 2011)

... and now I think about it, when you say

"All you need to do to add Option Explicit to each module is goto Tools>Options"

... making Option Explicit a default is a pefect example of orthodoxy. Once you turn this on as the default, then anywhere in your code, even for a simple thing, you are forced to declare your variable. 

There must be thousands of hours per year wasted in the world because of this setting alone!


----------



## Michael M (Oct 29, 2011)

And maybe.......thousands of hours wasted in the world because programmers can't find the code error, because they DIDN'T declare their variables !!


----------



## peterpre (Oct 29, 2011)

Michael M said:


> And maybe.......thousands of hours wasted in the world because programmers can't find the code error, because they DIDN'T declare their variables !!


 
Michael:

I agree. But I'm saying there is a trade-off. 

People also spend thousands of hours typing in informative comments and creating intelligent variable names - and many of these are never looked at again by anyone, ever.

The issue is an optimization problem.


----------



## Norie (Oct 29, 2011)

There's been a lot of hours wasted on this forum alone because someone hasn't used Option Explicit.

Common example x1Up and xlUp.

Without Option Explicit, x1Up won't stop the code compiling but it will break it at runtime.


----------



## Norie (Oct 29, 2011)

Peter

It does not take thousand of hours typing this information in.


----------



## peterpre (Oct 29, 2011)

Norie said:


> Peter
> 
> It does not take thousand of hours typing this information in.


 
Norie,

I  might be off, (I have been before!) but I based it on:

Population of US: 310 million
Number who are programmers: 1 in 500
Number of programers - absolute: 620,000
Coding hours per year per programmer: 2000
Total programming hours per year in US: 1.24 billion
Percentage of time spend on superflous programming: 3%
Hours of superflous programming: 37.2 million

Margin of error +/- several million.

But you're right - it's millions, not thousands.


----------



## Nalani (Oct 29, 2011)

> So question is: where is *the line* between simple and complex?


 <?xml:namespace prefix = o ns = "urn:schemas-microsoft-comfficeffice" /><o></o>
Here is my take on this subject, by the way this should be in the Lounge instead of Excel Questions.
1. You wrote the code
2. You understand it
3. You will maintain it
4. You will fix it when “broke”
5. It was written just for YOU
6. You could care less after you are gone

THE LINE_________________________________________THE LINE

1. You write the code in a more understandable way
2. As Norie stated, Option Explicit, Declarations, Variables, Comments, etc. all help long after it was written(years in some cases) to fix any problem or add to it
3. If writing code to help “others” the above is all good for their learning experience
4. Easily adapted once YOU are gone
<o></o>



> So the question is: have we got the right balance?


<o></o>
That is a question that only YOU can answer. What do YOU want to do or achieve.


----------



## gregtx81 (Oct 29, 2011)

I think Peter has to be trolling, at least partially.  

But on a related-to-formal-programming note, what type of environments do professional Excel-VBA programmers typically work in-- as consultants or within a large corporation?  Is their a concentration in any particular industry?  Are the applications predominantly front-ends to databases (SQL/Access)?

Do corporations have IT-help positions related to solely Excel?


----------



## peterpre (Oct 29, 2011)

Nalani said:


> <?xml:namespace prefix = o ns = "urn:schemas-microsoft-comfficeffice" /><o></o>
> Here is my take on this subject, by the way this should be in the Lounge instead of Excel Questions.
> 1. You wrote the code
> 2. You understand it
> ...


 
Agree about "should be in the lounge" and I like your "line graphic", but I think you've rather missed the point when you talk about "you this" and "you that".

The issue is about the complexity of the code and whether it justifies a certain degree of comment and variable-naming consistency. Not about who wrote it.


----------



## Jonmo1 (Oct 29, 2011)

If I have work I need done, I prefer to hire someone that does it right every single time, no matter how simple the job is.
And by "Does it right" I mean, follows the same rules and procedures in place for their job.  Every Time.


Take your car...
If you need some maintenance done (even a simple oil change).
do you take your car to Wal-Mart, where they find the quickest cheapest way to get the job done?
Or do you take it a mechanic you trust to do the job right every time?

I would rather spend the extra minute or so to declare my variable, then spend several minutes or hours chasing down a problem because I misspelled the variable.
Or Even worse, you didnt' realize you misspelled the variable and assumed the result of the code was correct, when it actually isn't.


----------



## peterpre (Oct 29, 2011)

jonmo1 said:


> If I have work I need done, I prefer to hire someone that does it right every single time, no matter how simple the job is.
> And by "Does it right" I mean, follows the same rules and procedures in place for their job. Every Time.
> 
> 
> ...


 
... but surely it's a trade off. If I only want milk, and Wal-Mart is cheaper, then I go for Wal-Mart (i.e, a non-complex coding module) but, as you say, if it's a more complicated requirement (oil -change = more complicated coding) then Wal-Mart might be a bad choice.

In other words: sometimes...

For x = 1 to 12

... is just fine with no declarations and no comments.


----------



## Jonmo1 (Oct 29, 2011)

Not sure what else to say.
You've gotten overwhelming responses to your question.
It's clear you have your mind made up, and no-one's going to change it.
And I don't think you're going to change anyone else's mind either.
So We're at an empass.


----------



## peterpre (Oct 29, 2011)

gregtx81 said:


> I think Peter has to be trolling, at least partially.
> 
> But on a related-to-formal-programming note, what type of environments do professional Excel-VBA programmers typically work in-- as consultants or within a large corporation? Is their a concentration in any particular industry? Are the applications predominantly front-ends to databases (SQL/Access)?
> 
> Do corporations have IT-help positions related to solely Excel?


 
I am seriously not trolling at all. And I think my question goes beyond just Excel-VBA programming.

Quote understandably, the convention is towards consistent variable naming and explicit comments.

All I am saying is that it has gone too far, so that in the small number of cases where such effort is not necessary, people still waste time following this convention.


----------



## taurean (Oct 30, 2011)

AA] Maybe this analogy shall let you decide:

At some of the traffic junctions, you see RED signal and you stop. And you see no one is crossing and mutter to yourself, what a waste of time, bloody waste of time indeed.

But that is the thumb rule that you are supposed to follow and someday, when a mishap occurs, following signals seems justified.

BB] In an environment, where more than one people are working on the same project, it actually pays off to have a convention and sticking to it for obvious errors and mixups.

CC] If you are doing it alone and probably you are the only one who is going to look after then perhaps you can cutdown as you want to. A chinese saying goes like this: Faded Ink is thousand times better than Sharp Memory. It roughly means the typing overload and waste that you feel you are doing now will come in handy otherwise you will be playing Sherlock Holmes for hours with your own code with No Watson around.

DD] Commenting is useful when you are using a new technique which you feel you won't remember next time when you open it. So don't do it like plague either that your code starts looking GREEN literally. But a dash of green will always help.

My 2 paise on this!


----------



## peterpre (Oct 30, 2011)

Overheard conversation last Wednesday at the “Post Road Tavern”…
<?xml:namespace prefix = o ns = "urn:schemas-microsoft-comfficeffice" /><o></o>
- So how’s business?<o></o>
- Pretty good actually – we’ve just got rid of some of our old processes and we’re saving a lot of wasted time.<o></o>
- Yeah?<o></o>
- Yeah. We looked at how we were spending our programming time and found that about ten percent was spent on putting comments into code.<o></o>
- So, what’s wrong with that?<o></o>
- Well, at first we thought that that was a good number. And we all swapped stories about the hours we’d spent tracking down errors when people had ignored the rules and there were no comments available.<o></o>
- Tell me about it!<o></o>
- But then we took another look and it turned out that these were the exceptions, and we found that in the vast majority of cases, comments were never read again. And then we said: this is like a poker game: What are the odds that the comments we write will be important in the future? If we spend too much time on comments, then we are wasting our time. If we do too few comments, then we will pay for it down the road, because we’ll spend a bucket-load of time trying to understand what we did when something goes wrong.<o></o>
- So what did you come up with?<o></o>
- Well, we decided that the worst policy was to have a “one-size-fits-all” approach that says: “you should always write comprehensive and explicit comments”. Instead, we said to our programmers - you decide if there is a reasonable chance that someone in the future will need to see any comments at all in this procedure. If you think so, then write a comment, if not, then don’t.<o></o>
- But isn’t that dangerous? What happens if you have a programmer who can’t make that type of judgment?<o></o>
- Well then the issue is more about our hiring capability – and we don’t think that we have a problem there … we know we’ve got an intelligent team.<o></o>
- But doesn’t just about every book say that you must consistently make comments in every case? Same thing when you go on forums.<o></o>
- Well, that’s why we hope we’re onto something. Let’s hope they keep thinking that way!<o></o>


----------



## Jonmo1 (Oct 30, 2011)

The example you used to state your case actually ended up proving the case against you....

You posted some seemingly extremely simple code...

But then Norie Said...


Norie said:


> Mind you I'd probably do it like this.
> 
> ```
> Range("A1") = 1000
> ...


You then had to explain why you wrote it that way..


peterpre said:


> ... although not if the user enjoys seeing the brief flicker of sequential numbers!


Seems a bit of comment in your sopposedly simple code would have elminated that question from Norie..
I noticed the same thing immediately as well.

I know it was just meant as an example, but there it is.

Just because the purpose of the code is simple and clear to you, doesn't mean it will be simple and clear to someone else.

In a proffessional environment, who knows when someone else will need to use/see the code.


Most programmers perfer to build habbits of doing things a certain way.
When you take shortcuts, you increase your chances of forgetting the correct way later on when it counts.
Not necessarily forgetting how to do it the right way, but just simply forgetting to do it.


It is far better to have and not need, then it is to need and not have.


And seriously, do you honestly believe 10% of the time is spent writing the comments and dimming variables?
I don't think so.
I'll spend more time on a coffee break than doing those little things.


----------



## MARK858 (Oct 30, 2011)

Why are you so concerned about the programmers typing time? I was always told that you declared variables to save system resources and to make the code more efficient/faster for the* end users* (please note the use of s at the end of users).
Or are you also proposing not using
*Application.DisplayAlerts = False
Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual*
to save the programmers typing time as the program will run without them.

On the subject of comments I use them to tell me what may need altering at a later date either by myself or by others, as a memory jog or to inform others who might have less knowledge what a line does(i.e. on a forum). 
I have never seen anyone say you should comment everything.

I would definitely put a comment in if I used a title for a macro like 





> Sub Rectangle1_Click()


 when the code had nothing to do with rectangles as I wouldn't have a clue what the code done 6 months later and would have to read the entire code again.


----------



## martindwilson (Oct 30, 2011)

anything that works suits me!


----------



## xlHammer (Oct 31, 2011)

It's actually number 3 of the Excel Commandments;

1. All pie charts are evil
2. Cell merging is for morons
3. The wisdom of variable declaration and code commenting is inviolable


----------



## RoryA (Oct 31, 2011)

peterpre said:


> The better coding approach is to decide what approach is required for each specific situation.



Was this meant to be a rhetorical question?

At the end of the day, for a trivial example such as what you posted, no it doesn't really make sense to add comments explaining _what_ it does, or using meaningful variable names for simple loop counters (many of us don't do that anyway). 
On the other hand, a single comment explaining _why_ the code is doing what it does, would be extremely helpful. If I came across that code in 6 months, my first thought would be "why is it doing that"?

I don't actually comment my code much (except sometimes as explanation if posting on a forum) but I do try to have a general comment as to its purpose. I figure anything beyond that won't be kept up to date, and if it's not clear _what_ a routine is doing, it probably needs to be broken up or simplified anyway.

I don't really accept the Option Explicit argument. If you don't have it as the default, then you have to remember to type it in when needed. Much simpler to have it default, then comment out temporarily if required. Of course, following your paradigm, I'd have to have separate modules for complicated routines and easy ones, rather than simply grouping by purpose. I'd also have to waste time deciding whether a piece of code fell on the complicated side of the line or not.

Anyway, if we were really worried about efficiency and hours spent, we wouldn't be *here*, now would we?


----------



## SydneyGeek (Oct 31, 2011)

I'd apply the 6 month rule. If it's not clear immediately what you did when you come back to it, it should have been commented.

I always use Option Explicit
For quick and dirty, use-and-dispose code, I don't comment.
For everything else I write a simple explanation at the top of the routine, and other comments as required. If I am about to write something big I 'wireframe' it wth comments, then write the code to match those comments.

Denis


----------



## xenou (Oct 31, 2011)

> I'd apply the 6 month rule. If it's not clear immediately what you did when you come back to it, it should have been commented.



Sounds fair to me.  Of course you only discover what this really means after coming back to code a few times after six months to discover you have no idea anymore ...  while the fact that I *have* done just that is evidence that I don't always follow the so-called inviolable commandments... 

I'll take good code any day no matter what the style.  I've seen enough bad code with nicely declared variables to convince me that what I like to see in a program is a good programmer behind it.  If you have code that you expect to use and maintain for many years, then its well worth time documenting it.  As far as your original example goes, I wouldn't need the comments for such a case so no problem there.  Much of my code is broken into functions and subs, usually with a kind of "plot summary" in the comments that tell me what I'm doing more than how it works - when I review the code I read the comments to understand what it is meant to do so I can follow the flow quickly and figure out where I want to put my attention.  Since I like to break up code into functions and subroutines, "meaningful procedure names" can be a form of self-documentation.  

I use a lot of {i, j, s, t, a, x} variables for simple stuff in functions of 10-20 lines.  After that I start getting more careful about meaningful identifiers.  This is especially helpful in VBA where we don't have block level scope (too bad - I'd really love that).


----------



## diddi (Nov 1, 2011)

peterpre, you really do display just a tiny bit of ignorance, this being your first thread on the forum. i support various comments from the regulars (eg rorya) and would add my perspective as follows.

i use sensible variable names and subroutine names which often do not require many extra comments as a result.  i write generic subroutines which i regularly recycle which saves me a lot of precious seconds (LOL) and i group related code into seperate modules. oh! - and i religiously indent my code. you have done none of these things in your example.

when i have to revisit an old project (because nothing ever remains static in terms of user demands) i know ill see the same sort of structure and code that i used last week. quick to modify or debug.


----------



## RoryA (Nov 1, 2011)

diddi said:


> peterpre, you really do display just a tiny bit of ignorance, this being your first thread on the forum.



I can't see any display of ignorance.

It's a good thing to have different perspectives on a topic and to challenge conventional wisdom. Anything that doesn't stand up to a bit of scrutiny isn't really worth following, after all.


----------



## Michael M (Nov 1, 2011)

> It's a good thing to have different perspectives on a topic


True, but I'm starting to feel like this has been done to death !
I didn't understand where this thread was heading right from the start, once a couple of boundaries / opinions had been made the whole thing became moot .....but, we press on.


----------



## Domski (Nov 1, 2011)

None of us (yes I know, even me) writes consistently perfect code so a little bit of time declaring variables and adding comments for the time it saves at a later date is worth every second imho.

Dom


----------



## arkusM (Nov 1, 2011)

Domski said:


> None of us (yes I know, even me) writes consistently perfect code...
> Dom


 
WHAT?!?!??! the pillars of this forum are mere mortals... my illusions are shattered... 



			
				diddi said:
			
		

> ...and i religiously indent my code...


 
diddi, you are using this right? http://www.oaltd.co.uk/Indenter/indentpage.asp (I guess if you can, due to corp. policies etc...)
Helps me find missing "End __'s" and the like.


----------



## litrelord (Nov 3, 2011)

I worked with a guy who had a very similar thought process about not commenting or declaring veriables. I got lumbered with picking up the pieces after he'd left and it was a horrible job to go through and understand what everything was meant to be doing. Some of the code didn't work at all how you'd expect because of a variable that had been typed with fat fingers.

On the plus side I got to tell everyone that although there had been some problems with the data they'd been receiving I'd now fixed it and it would work properly so I should probably thank him for making me look good. 

So to reiterate everyone else's comments, yes there are occasions when I might delete Option Explicit and not declare or comment but if it's for a production then I'll comment and declare properly as I'm arrogant enough to believe that it will still be used in years time when I'm no longer responsible for it 

Nick


----------



## NateO (Nov 3, 2011)

I'm going to go with Option C. A is lazy, at best, and you don't really want to use an Integer, in either case, which A coerces your numbers to. And B actually forces the issue to be an Integer. On a 32-bit system, you'll want a Long.

You can get into a lot of trouble by not declaring your Variables, try using DAO and ADO in Access sometime:

http://access.mvps.org/access/bugs/bugs0031.htm

Taking 2 seconds to declare your Variables properly will save you a headache. It's good practice.


----------



## mortgageman (Nov 3, 2011)

xenou said: 



xenou said:


> <BIG Snip> This is especially helpful in VBA where we don't have block level scope (too bad - I'd really love that).


 
I got curious about this so I did some (very little) research on "block level scope". This link sorta convinced me (mind you, I know a LOT LESS then you about this type of stuff) that BLS would be a pretty bad idea. What are your thoughts about this?

http://visualbasic.about.com/od/quicktips/qt/blkscp.htm 

Gene Klein
</BIG>


----------



## NateO (Nov 3, 2011)

Does VBA not have block-level scope? My test here seems to indicate it does, to some extent:


```
Sub foo()
Dim cl As Range
For Each cl In Range("A1:A3")
    Debug.Print cl Is Nothing
Next
Debug.Print cl Is Nothing
End Sub
```
 
In terms of intrinsic data types, that's a one-liner, e.g.,


```
Sub bar()
Dim i As Long
For i = 1 To 3
    Debug.Print i
Next i
Let i = Empty
Debug.Print i
End Sub
```


----------



## shg (Nov 4, 2011)

Empty is a value reserved to Variants.

Assigned to a numeric variable, it gets converted to zero. It doesn't release any storage in either case, unless assigned to a variant array. Variables (scalers, scalar arrays, objects, ...) are released when they go out of scope.


----------



## jeffreybrown (Nov 4, 2011)

I agree with all the points about Option Explicit, Declaring Variables, indenting code, commenting code, etc.

By no means do I consider myself a programmer, but as a member of this board I have learned some invaluable tips and not one them I can think of which would cause any extra ordinary time in coding practices.

When I was just starting out learning how to use code, I would get some code from somebody who did not use Option Explicit, but since my default was Option Explicit, I spent time scratching my head trying to declare all the variable which were left out.  I felt this was by far a better way to understand the code than just removing Option Explicit.

Just today I used a little snippet of code I have saved and realized if I updated the code just slightly it would be a little more efficient in the updating department.  Probably something else could be done, but this gets me a little further down the road.  With the before code, if you change the column you have to update it three times where in the after just once.  May seem minuscule in the big picture, but every bit helps.

Before:

```
Sub concat1()
    Dim LR As Long
    LR = Range("A" & Rows.Count).End(xlUp).Row
    Range("B1").Value = Join(Application.Transpose(Range("A1:A" & LR)), ", ")
End Sub
```

After:

```
Sub xpos()
    Dim LR As Long
    Const ColLet As String = "A"
    Const SR As Long = 1
    Dim rngDst As Range
    Set rngDst = Range("B1")
    LR = Range(ColLet & Rows.Count).End(xlUp).Row
    rngDst = Join(Application.Transpose(Range(Cells(SR, ColLet), Cells(LR, ColLet))), ", ")
End Sub
```

FWIW...my 2-cents... 

BTW, using the Const variable in the code is by way of Rick Rothstein and I thank him and the many others on this Forum for sharing their wonderful talents and experience.


----------



## JamesW (Nov 7, 2011)

On a, slightly, unrelated note: http://xkcd.com/974


----------



## RoryA (Nov 7, 2011)

jeffreybrown said:


> BTW, using the Const variable in the code is by way of Rick Rothstein and I thank him and the many others on this Forum for sharing their wonderful talents and experience.



FYI, Const is, by definition, _not_ a variable.


----------



## diddi (Nov 9, 2011)

arkusM said:


> diddi, you are using this right? http://www.oaltd.co.uk/Indenter/indentpage.asp (I guess if you can, due to corp. policies etc...)
> Helps me find missing "End __'s" and the like.



hi arkusM,   no i dont use any utilities for indenting.  its just a way i find that works for me to match up all the nesting.  if i get to the end of a code fragment and things dont line up i can be pretty sure that i have missed an End If or something. i guess it forces me to work in small logical blocks and not get too unruly.

i did try an addin once for indenting but i found it more of a pest than a help. cant remember that name of it now. lol

oh, and a thought about variable names, for what its worth: i often use an uppercase char when i declare my variable names.  eg MyVar  , the benefit being that when i have an attack of digital dyslexia, the variable doesnt autocapitalise (eg myvra) and then i know i need to fix it up.




good to see some lively discussion in the lounge!


----------



## Norie (Nov 10, 2011)

diddi

You really should try SmartIndenter which I think is the add-in at the end of the link.

It's certainly not a pest - right click, select what you want to indent(Procedure, Project, Module) and that's it.

You can also set various options, eg align Dim's in a column (which I've never used)

It's certainly handy when copying data from here to find the source of that elusive Next before For error and the like.

Oh, forgot to add - when it's installed it's installed in all of Office.


----------



## Greg Truby (Nov 10, 2011)

This thread is pretty mature by now and I reckon most POV's have been covered. So, Peter, if you're still monitoring this, I admit that when I read your question, I immediately made several assumptions and I confess I'm curious to know whether I'm right...

How long have you been writing code? Have you ever had to go back and try and decipher code you wrote five, ten, fifteen or even twenty years ago? 

Have you ever had to try and unravel mission critical code that suddenly quit working after some software upgrade or hardware change and the code was poorly documented (if it was documented at all)? Or had to sift through someone else's code and try to comprehend it when it was written with all one-letter variable names? All while you're getting hourly e-mails from senior management asking "how's it going"?

Have you ever had to step through hundreds of lines of code hunting down some "magic value" that is embedded deep in code in several different procedures because the programmer failed to use a module-level or global constant? (Which, of course, is doubly sad when you are the nitwit who wrote the code ten years back, before you knew better.)


----------



## arkusM (Nov 10, 2011)

Greg Truby said:


> Have you ever had to step through hundred of lines of code hunting down some "magic value" that is embedded deep in code in several different procedures because the programmer failed to use a module-level or global constant? (Which, of course, is doubly sad when you are the nitwit who wrote the code ten years back, before you knew better.)


 
Doh, busted... This happens to me, I am glad I have started to be better thanks to the kind folks here...

diddi, check out smart indenter, time saver, not a crutch, well ok it could be.... but I can affirm what Norie said.


----------



## Domski (Nov 11, 2011)

I've only just started using the Smart Indenter after ages trying to persuade IT to let me install it at work and have to admit it's very useful and saves a lot of time moving code around. Even though I don't think it says on the website it works fine in Excel 2007/2010 as well.

Dom


----------



## dk (Nov 11, 2011)

Ditto on the Smart Indenter - cannot do without it.  I've now got the keystrokes down to a split second fine art of indent project followed by Debug, Compile everytime I'm ready to save.  It's a shame that Mr Bullen seems to have not added anything new to his site for a while as he's done some brilliant work although I guess a man of his skill would be plenty busy.  <a href="http://www.oaltd.co.uk/DLCount/DLCount.asp?file=FormFun.zip">FormFun.zip</a> is another of my favourites of his.


----------



## diddi (Nov 12, 2011)

so a thumbs up for SmartIndenter?  i will give it a go, on your advice.  esp. as norie says that it has preferences. all good. well done to all for their input.

just a bit of a survey question:  does anyone indent Open/Close 'nests'?  i used not to but found it quite useful


----------



## Norie (Nov 12, 2011)

What sort of Open/Close 'nests' do you mean?


----------



## diddi (Nov 13, 2011)

when i am doing random file operations, for example.  it just reminds me that i have a file open, so dont forget to close it and then wonder why, 2 months later, my latest sub randomly fails with file already open errors.

not meant to be rocket science, but it helps me make a few less blunders.


----------



## Norie (Nov 13, 2011)

diddi

If you mean File I/O then yes I would indent it.


----------



## dk (Nov 14, 2011)

Indenting that sort of thing is a little tricky - Smart Indenter doesn't support it as it's not something that you have to do in order to get it to compile (unlike control structures such as If Then, For Next and Select Case).  And there's nothing stopping you opening a file in one procedure and closing it in another (although this would probably not be ideal).

One neat way I've found of handling resources such as this is to create a class module that does the work for you e.g. opens the file, reads, writes and closes but you make sure that you put the Close code in the class's Terminate event.  I've been using this for database connections and it works a treat.  As soon as the object goes out of scope the connection gets closed and the associated object variables get destroyed.  It's actually quite easy to implement and you can then re-use in any project quite easily.  The benefits are that even though you plan to code correctly and close every file/database connection if you happen to forget or if your code errors and your error handler is not designed properly then the resource will still be closed off correctly.

EDIT: this was not my idea but I read it in Scott Meyers Effective C++ book.  The same technique can also be applied to VBA.

HTH
DK


----------



## Greg Truby (Nov 14, 2011)

dk said:


> ...One neat way I've found of handling resources such as this is to create a class module that does the work for you e.g. opens the file, reads, writes and closes but you make sure that you put the Close code in the class's Terminate event.


 
Hmmm, now I'm gonna have to experiment with this when I get the chance. I'd seen it before in other apps, but had not adopted it as a general practice. Though I probably will going forward.

In particular - I have an Excel add-in that has about 80 users in the client's company and I've had an issue where if a user crashes Excel at a particular spot they have a common file open and locked and then subsequent users cannot open the file and it raises errors. I have no idea if the Terminate code for the class module will run during an Excel crash. I rather suspect it won't. But certainly worth playing with. The hard part is reliably crashing Excel. (I'm not soliciting help on this - just saying why it caught my eye so. But if anyone has a brilliant solution, feel free to post it for the public's edification.)


----------



## Jon von der Heyden (Nov 14, 2011)

I have adapted this code for a number of projects to check (1) if a workbook is open and (2) who has it open.


```
Sub TestVBA()
'// Just change the file to test here
Const strFileToOpen As String = "C:\Data.xls"

    If IsFileOpen(strFileToOpen) Then
        MsgBox strFileToOpen & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileToOpen), vbInformation, "File in Use"
    Else
        MsgBox strFileToOpen & " is not open"
    End If
End Sub

Function IsFileOpen(strFullPathFileName As String) As Boolean
'// VBA version to check if File is Open
'// We can use this for ANY FILE not just Excel!
'// Ivan F Moala
'// http://www.xcelfiles.com

Dim hdlFile As Long

    '// Error is generated if you try
    '// opening a File for ReadWrite lock >> MUST BE OPEN!
    On Error GoTo FileIsOpen:
    hdlFile = FreeFile
    Open strFullPathFileName For Random Access Read Write Lock Read Write As hdlFile
    IsFileOpen = False
    Close hdlFile
    Exit Function
FileIsOpen:
    '// Someone has it open!
    IsFileOpen = True
    Close hdlFile
End Function

Function LastUser(strPath As String) As String
'// Code by Helen
'// This routine gets the Username of the File In Use
'// Credit goes to Helen
Dim text As String
Dim strFlag1 As String, strflag2 As String
Dim i As Integer, j As Integer

strFlag1 = Chr(0) & Chr(0)
strflag2 = Chr(32) & Chr(32)

Open strPath For Binary As #1
    text = Space(LOF(1))
    Get 1, , text
Close #1
j = InStr(1, text, strflag2)
i = InStrRev(text, strFlag1, j) + Len(strFlag1)
LastUser = Mid(text, i, j - i)

End Function
```


----------



## Greg Truby (Nov 14, 2011)

Hi Jon,

I was excited about the idea of being able to identify who had a file open. But I'm getting dodgy results with that LastUser function. 

If the file in question is a workbook opened in another instance of Excel (test #2) LastUser() Overflows on the integers - easily fixed by declaring i and j and longs. But then it returns a bit of readable text and a bunch of non-readable characters.


```
Sub TestVBA()
    '// Just change the file to test here
    Const strPath           As String = "C:\Users\Greg\Documents\Excel\VB Playground\", _
          strFileToOpen1    As String = "book1.xls", _
          strFileToOpen2    As String = "RibbonTest.xlsm", _
          strFileToOpen3    As String = "dropdown3.txt"
    Dim strFileName As String
    '// (workbook - not opened)
    Let strFileName = strPath & strFileToOpen1
    If IsFileOpen(strFileName) Then
        MsgBox strFileName & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileName), vbInformation, "File in Use"
    Else
        MsgBox strFileName & " is not open"
    End If
    '// (workbook - opened in a different instance of excel)
    Let strFileName = strPath & strFileToOpen2
    If IsFileOpen(strFileName) Then
        MsgBox strFileName & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileName), vbInformation, "File in Use"
    Else
        MsgBox strFileName & " is not open"
    End If
    '// text file opened here
    Open strFileToOpen3 For Output As #2
    Let strFileName = strPath & strFileToOpen3
    If IsFileOpen(strFileName) Then
        MsgBox strFileName & " is already Open" & _
            vbCrLf & "By " & LastUser(strFileName), vbInformation, "File in Use"
    Else
        MsgBox strFileName & " is not open"
    End If
    Close #2
End Sub
```
For test #3, LastUser() errors out on the Open ___ For Binary since the file is already open. I tried editing it to be:

```
Open strPath For Binary Access Read As #1
```
but that still errors out.


----------



## Greg Truby (Nov 14, 2011)

As an addendum, the following function does appear to work, but I only tested it lightly and only under Win7. Various versions seem to be floating around the internet. I pulled this from ozgrid, but not sure who is the original author
	
	
	
	
	
	



```
Function GetFileOwner(fileName As String) As String
 
    Dim secUtil As Object
    Dim secDesc As Object
 
    Set secUtil = CreateObject("ADsSecurityUtility")
    Set secDesc = secUtil.GetSecurityDescriptor(strfilename, 1, 1)
    GetFileOwner = secDesc.Owner
 
End Function
```
 
EDIT - I always like to try an early bound version when playing around. Here's what that looks like:
	
	
	
	
	
	



```
Function GetFileOwner(fileName As String) As String
    
    '// to use early bound, set reference to
    '// Active DS Type Library
    
    Dim secUtil As ActiveDs.ADsSecurityUtility, _
        secDesc As ActiveDs.SecurityDescriptor
    
    Set secUtil = New ActiveDs.ADsSecurityUtility
    Set secDesc = secUtil.GetSecurityDescriptor(fileName, 1, 1)
    GetFileOwner = secDesc.Owner
     
End Function
```


----------



## diddi (Nov 14, 2011)

File I/O in a multiuser environment has presented problems for me for a while.  i have resorted the the not so ideal method of making very short references to files, keeping them open for the least amount of time possible.  this works ok for my needs, but if there is a Class level method for file I/O which can help in multiuser context i would be very keen to see more (hint )


----------



## dk (Nov 14, 2011)

diddi said:


> File I/O in a multiuser environment has presented problems for me for a while.  i have resorted the the not so ideal method of making very short references to files, keeping them open for the least amount of time possible.  this works ok for my needs, but if there is a Class level method for file I/O which can help in multiuser context i would be very keen to see more (hint )



Hi mate

I'm actually talking about creating your own class to handle the file I/O but then utilise the class's terminate event to ensure that it gets closed properly.  Here's an example which should hopefully illustrate the idea.  I'm assuming that the file I/O you're referring to is the normal Open, Close, Print, Write, etc but let me know if not.

Insert a class module and name it CFile and paste this code:


```
Option Explicit

Private Const mlCLASS_ERROR As Long = vbObjectError + 1
Private miFreeFile As Integer
Private mbFileOpen As Boolean

Public Function OpenFile(sFilename As String)

    On Error GoTo ErrHandler

    If Not mbFileOpen Then
        miFreeFile = FreeFile()
        Open sFilename For Output As #miFreeFile
        mbFileOpen = True
    Else
        Err.Raise mlCLASS_ERROR, , "File already open."
    End If


ExitFunction:
    Exit Function

ErrHandler:
    Err.Raise mlCLASS_ERROR, , Err.Description

End Function

Public Sub WriteLine(sValue As String)

    If Not mbFileOpen Then
        Err.Raise mlCLASS_ERROR, , "File not open."
    Else
        Print #miFreeFile, sValue
    End If
End Sub

Public Sub CloseFile()
    Close #miFreeFile
    mbFileOpen = False
End Sub


Private Sub Class_Terminate()
    On Error Resume Next
    CloseFile
End Sub
```

Then to test it use something like this:


```
Sub TestIt()
    
    'Test of CFile class

    On Error GoTo ErrHandler

    Dim oFile As CFile

    Set oFile = New CFile

    oFile.OpenFile ("C:\temp\hello.txt")

    oFile.WriteLine "test file written by CFile object..."
    oFile.WriteLine "another line..."
    oFile.WriteLine "closing now..."
    oFile.CloseFile

    MsgBox "File succesfully created.", vbInformation, "CFile Test"

ExitSub:
    Exit Sub

ErrHandler:
    MsgBox Err.Description, vbExclamation, "An error occurred."

End Sub
```

With this technique the file gets closed regardless of whether your code errors out (e.g. trying to create a file on a non-existent folder) or if you don't explicitly call the CloseFile method.  You can test this by putting a Msgbox in the class's CloseFile method and then commenting out the oFile.CloseFile.

HTH
DK


----------



## diddi (Nov 14, 2011)

@dk

ah yes i see how that works.  must admit i would not have thought about tackling the problem like that, but i shall give it a try.
thx


a question:   does the "File already open error" refer to the current code already having the file open, or another user having the file open?


----------



## dk (Nov 15, 2011)

diddi said:


> @dk
> a question:   does the "File already open error" refer to the current code already having the file open, or another user having the file open?



It means that the CFile object can only have 1 file open at a time.  If you wanted multiple files then you'd just use multiple CFile objects i.e. 1 CFile object = 1 file.


----------



## Jon von der Heyden (Nov 15, 2011)

Greg Truby said:


> As an addendum, the following function does appear to work, but I only tested it lightly and only under Win7. Various versions seem to be floating around the internet. I pulled this from ozgrid, but not sure who is the original author
> 
> 
> 
> ...



Thanks for sharing Greg.    I wasn't aware of the quirks in the previous function; having only ever used it on excel workbooks within the same instance.  Nice to have someone else do all the hard work for me.


----------



## dk (Nov 15, 2011)

Greg Truby said:


> Hmmm, now I'm gonna have to experiment with this when I get the chance. I'd seen it before in other apps, but had not adopted it as a general practice. Though I probably will going forward.
> 
> In particular - I have an Excel add-in that has about 80 users in the client's company and I've had an issue where if a user crashes Excel at a particular spot they have a common file open and locked and then subsequent users cannot open the file and it raises errors. I have no idea if the Terminate code for the class module will run during an Excel crash. I rather suspect it won't. But certainly worth playing with. The hard part is reliably crashing Excel. (I'm not soliciting help on this - just saying why it caught my eye so. But if anyone has a brilliant solution, feel free to post it for the public's edification.)



G'day Greg

If you're talking about Excel at crashing as opposed to a run-time error then you're right that the class technique is not going to help.  I found <a href="http://www.mrexcel.com/forum/showpost.php?p=69672&postcount=5">some code</a> for crashing Excel (use with caution!) and tested if the terminate event got called.  Unsurprisingly it was not called so not sure how you could reliably handle the situation you've got.

Cheers
Dan


----------



## Greg Truby (Nov 15, 2011)

Hi Dan,

I my specific context, I was able to code around the problem. It wasn't particularly elegant or pretty; but it was also a rare event, so I didn't want to invest an excessive amount of time on it. Nonetheless, just in general terms it would be nice to have a solution path for a "crash left file locked" situation. I'll certainly play with everything we've mentioned here when I get the chance.

Just curious - do you go ahead and base all your custom error off _vbObjectError_?  I know it's the "right thing to do" but I just have a completely irrational dislike for it because it's a large negative number.  I just start mine at 1100 (and yes, I know there's a danger that someday MS will use those numbers).

@ diddi, I've also seen (and I would expect dk has too) versions of the "generic file" class that have an optional access/mode argument.


----------



## dk (Nov 15, 2011)

Greg Truby said:


> Just curious - do you go ahead and base all your custom error off _vbObjectError_?  I know it's the "right thing to do" but I just have a completely irrational dislike for it because it's a large negative number.  I just start mine at 1100 (and yes, I know there's a danger that someday MS will use those numbers).



To be honest, no.  That class code was something i quickly typed up to demonstrate the idea.  Nearly all of my code uses the error handling techniques from the Professional Excel Development Handbook - I have just looked at one of the modules and they have defined the error code as:

Public Const glHANDLED_ERROR As Long = 9999

So not even a mention of vbObjectError.  This has me confused now - according to the help it says:

<i>Required. Long integer that identifies the nature of the error. Visual Basic errors (both Visual Basic-defined and user-defined errors) are in the range 0–65535. The range 0–512 is reserved for system errors; the range 513–65535 is available for user-defined errors. When setting the Number property to your own error code in a class module, you add your error code number to the vbObjectError constant. For example, to generate the error number 513, assign vbObjectError + 513 to the Number property.</i>

However, if I do this I do not get the 513 as a result:


```
Sub blah()

    Const z As Long = vbObjectError + 513

    On Error GoTo errHandler

    Err.Raise z, , "custom error"


    Exit Sub

errHandler:
    MsgBox "error code: " & Err.Number & ", Desc: " & Err.Description

End Sub
```

I get "error code: -2147220991, Desc: custom error" so no mention of error code 513 so not sure what's going on there.  Any ideas?

Dan


----------



## Greg Truby (Nov 15, 2011)

dk said:


> I get "error code: -2147220991, Desc: custom error" so no mention of error code 513 so not sure what's going on there. Any ideas?


 
Well, you get that value because _vbObjectError_ is equal to -2147221504.

The way I interpret what's in help is "we [MS] will never use an error code [reasonably] north of _vbObjectError,_ so use that as your base and go from there", i.e.


```
Public Enum ge_ProjectErrors
    '// general
    errUserAborted = vbObjectError + 1
 
    '// used by basXL_CommandBars
    errControlCreationFailure
    errBarResetBackLoopUnknown
 
    '// used by basXL_OpenClose
    errXLVersionTooLow
    errMyDocsPathIsNetworkPath
 
    '// used by clsExcelApp
    errNoWorkbookObj_Allocation
    errNoWorkbookObj_Merchant
    errNoWorkbookObj_PRC
    errNoWorksheetObj_Allocation
    errNoWorksheetObj_Merchant
    errNoWorksheetObj_PRC
    errVrsnTooLow
 
    '// used by basXL_EntryPoints
    errBadMenuParam
    errBadTemplateVersion
    errInvalidDataDetail
    errInvalidDataHeader
    errNoDetailData
    errNotInProject
    errUnknownTemplate
'...
End Enum
```
 
Like I said, my objection is purely emotional: -2147221504 just comes across as so dad gummed arbitrary although it is a little prettier in hex (FF80040000).


----------



## RoryA (Nov 15, 2011)

You (the consumer of the object) have to remove the vbobjecterror again to get a true error number. IMO this is more relevant to VB programmers building dlls that ought to follow conventions (I think the error number is actually a couple of specific error bits that would have more meaning to C++ programmers, IIRC). For VBA I think it's irrelevant as long as you stay out of the MS error number range.


----------



## dk (Nov 15, 2011)

rorya said:


> You (the consumer of the object) have to remove the vbobjecterror again to get a true error number. IMO this is more relevant to VB programmers building dlls that ought to follow conventions (I think the error number is actually a couple of specific error bits that would have more meaning to C++ programmers, IIRC). For VBA I think it's irrelevant as long as you stay out of the MS error number range.



I see.  That sort of ties in with what I read here yesterday: http://vb.mvps.org/hardcore/html/howtoraiseerrors.htm - although I have been using C++ for the last year I have yet to do any COM stuff (just XLLs and stand alone executables) so this will be something for me to look forward to.

Cheers


----------



## dk (Nov 15, 2011)

@greg - yes mate, I understand that the number is negative because of vbObjectError's value - what I couldn't understand is that the help said that if you add 513 to vbObject error it will return error code 513 - although that would mean that vbObjectError would be zero.  I think Rory's point about having to remove the vbObjectError to get the "real" error code makes sense though.

BTW, your comment about about the value being prettier in hex made me chuckle  

Cheers


----------



## Greg Truby (Nov 15, 2011)

Dan,

Thanks so much for that link.  It certainly helps explain why _vbObjectError_ looks so much purdier in hex than in decimal.  :wink:


----------

