Let's clean up the old procedure names to be consistent
byronwall opened this issue · 5 comments
Since this code started as a hodge podge collection of things, there is little consistency on procedure names. At times, I thought it was a good idea to include some leading part to indicate which module contained the code.
At another time, I considered creating class modules for different functions and forcing namespaces that way. I never got too far down this path which is probably good.
I think for procedure names, my main goals are:
- The name clearly indicates what the procedure does. I'd favor a long name over a short one if it makes the description unambiguous. Ex:
CombineCells
could be improved.ColorForUnique
gets close,SelectionOffsetToRight
is probably good. - The name should probably be
UpperCamelCase
or whatever they call it with the leading capital letter. I think this looks much better thanSomething_AndThenUpperCamelCase
.
Related to code organization, I would be interested in seeing a better breakdown of procedures into new/different modules. I think there is no real penalty for extra modules, but they can go a long way to organizing code.
Any thoughts on this?
You're right, there's no penalty for modules (unless it gets ridiculous). I'm not really good with class modules myself. I do agree that the names should be descriptive and I like UpperCamelCase. The snag is that renaming will break ribbon calls (and other calls) unless they are changes as well. So doing changes might make sense to load everything into one file, and find and replace the title you're changing.
Right now there are 5 modules for chart events. These could probably be combined as charting tools. For all the chart modules, there are 2 functions. You could break the functions out to chart functions and the rest to subroutines. But, I'm not loving that idea.
I do like moving functions away from subroutines, it makes them easier to find when troubleshooting. There are 5 (non chart) functions right now, but I imagine as the code grows more functions will result.
Right now I sometimes can't find what I'm looking for when something is called from the ribbon. When FillValueDown
is called - I look to formatting, but it's in usability. That's strange to me - I understand it's because of the cobbling, we've all cobbled together code.
I did my best to find all the ribbon calls that could be broken and need error handling, that was sort of a difficult process - especially with the forms sometimes. The forms, in my head, make perfect sense in the code they possess - as long as the code is only accessed by the form.
Right now there are routines that make a change to the sheet and others that make a change to the data. So coloring and locking both don't change the data. But deleting hidden does, as well as convert to number. That might be one way of separating the subs. But honestly, I'm not sure either.
Regarding finding a Sub from another one, you can right click where it is called (like in Ribbon_Callbacks
) and choose Definition
. This is a built in VBE thing. It will jump to the declaration for that piece of code if it exists. SHIFT+F2 will also call that if you're into keyboard shortcuts.
Splitting Subs and Functions would be a good idea.
I think some gain will also come from removing code that exists but is not used anywhere.
Maybe we put this off for a little bit until we have gone through and removed code that is not being used. From there, we can decide how best to split things out.
It's possible the code written for #64 can be adjusted to modify procedure and function names with consistency.
I think procedure names will be correctly handled by Rubberduck renaming now. I haven't test it yet, but I think that add-in has gotten good enough to do this.
The main thing is to choose a consistent naming scheme that makes sense and can be applied across all the code once.
I removed my previous comment for.. just so many reasons.