search

byronwall / bUTL

Excel add-in with helpers for charting, formatting, and general pain points
http://byroni.us/bUTL
MIT License
16 stars 3 forks source link

Adhering to VBA Best Practices #48

Closed RaymondWise closed 8 years ago

RaymondWise commented 8 years ago

So, I recently saw a few things that were interesting and may need to be standardized within the add-in -

  1. Integers - integers are obsolete. According to msdn VBA silently converts all integers to long.
  2. Standard VBA naming conventions have camelCase for local variables and PascalCase for module variables and function and SHOUTY_SNAKE_CASE for constants. I think there's a lot to be gained here.
  3. When you don't define your variable, VBA will declare it as a Variant, which are objects: A variable you declare with the Object type is flexible enough to contain a reference to any object. However, when you invoke a method or property on such a variable, you always incur late binding (at run time). To force early binding (at compile time) and better performance, declare the variable with a specific class name, or cast it to the specific data type. (I think it's good here, but it was interesting enough to note)
  4. All arguments not passed ByVal are passed implicitly ByRef by default. Public is also implicit.
byronwall commented 8 years ago

Good stuff. Thanks.

Completely agree on naming conventions. I have thoroughly hacked together every possible naming scheme in the creation of the add-in. I may have invented a couple as well. It's bad. With the help of Rubberduck, I think renaming these should be straightforward enough. Just need to start doing it. I'll read through that link to make sure I rename them to appropriate names the first time through.

Wish I had known that about ints and longs. I have wasted a lot of time on another project because of overflowing Integers. Must be a bad habit to think hard about how large of a number I want to store and declare appropriately. I'll quit doing that now in VBA.

I was generally aware of the early/late binding. The main issue with early binding is that you have to declare a proper reference in the VBE before it will work if the object is an external one. I have had this create problems before (think it was related to someone saving the add-in/file who didn't have the reference and then it erased the reference?). Either way, I think I use the Dictionary in here and most everything else is bound late. Most things are internal objects anyways. Worth a review.

I generally want ByRef so I never think about it. I always assumed that value types were passed ByVal by default but guess not. I'll have to test that. I think it's generally good practice to not change the parameters that come in to a Sub so it's probably a moot point. Could definitely be the source of a hard to find bug though if you accidentally change a parameter that you thought was ByVal.

byronwall commented 8 years ago

PR #49 addresses some of these issues and put's us in a much better spot with respect to naming conventions.

RaymondWise commented 8 years ago

https://github.com/byronwall/bUTL/pull/61 rounds this out

byronwall commented 8 years ago

Thanks for following through on this. I'll close this one and we can handle anything that remains with new issues.