Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let's clean up the old procedure names to be consistent #28

Open
byronwall opened this issue Aug 6, 2015 · 5 comments
Open

Let's clean up the old procedure names to be consistent #28

byronwall opened this issue Aug 6, 2015 · 5 comments

Comments

@byronwall
Copy link
Owner

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 than Something_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?

@RaymondWise
Copy link
Contributor

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.

@byronwall
Copy link
Owner Author

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.

@RaymondWise
Copy link
Contributor

RaymondWise commented Nov 2, 2016

It's possible the code written for #64 can be adjusted to modify procedure and function names with consistency.

@byronwall
Copy link
Owner Author

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.

@RaymondWise
Copy link
Contributor

I removed my previous comment for.. just so many reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants