Advice please on good practice

[smiler44]

Is it better to put all the code in one sub routine or use a call statement to run other sub routines?
I am concerned that putting all the code in one sub routine with lots of if and end if can become difficult to read and follow.

If it is better to use different sub routines, do I put all these sub routines in one module or use a number of different modules?

thanks

smiler44

 

[MajP]

A lot of this is subjective and deals with readability, maintainability, ease of use, and ability to debug. You can go to either extreme.
I tend to have a lot of modules if they are a related group of methods (printUtilities, applicationUtilities, FormUtilities, ImageUtilities etc...)
But I do not just add extra modules based on size

I tend to have lots of very short procedures and functions because that is readable to me. Other people tend to like to see everything in one place.
So it is an art more than a science.
I can look at code and know it when it is bad and hard to read. But different people can organize differently and get clear code. The way my mind thinks makes sense with many modules and small procedures that do one thing.

 

[Gasman]

I myself would have one routine per task.
Then if I needed to run 3 out of 5 tasks, I would call the 3 routines.

 

[The_Doc_Man]

The general rule in ANY part of programming, whether we are talking databases, complex web sites, or some utility program, is "Divide and Conquer." There is a point where dividing up a subroutine goes too far, but from a pragmatist's point of view, "putting all the eggs in one basket" isn't such a good idea either.

A question you must ask is "can I use some part of this routine in more than one context?" Code-reuse is always a good thing if it is possible.

Some folks have a rule of thumb that says, if my routine's code won't fit on one screen, I have to split it into page-sized units. Other folks put limits like "no more than 100 lines". These are all self-imposed standards, or perhaps simply guidelines. You do what you have to do - but look for cases where you can isolate some routine to a very simple sequence, because that makes debugging easier when you can isolate code parts to become separate routines. Ease of debugging is NOT a trivial reason to split code into distinct parts.

As to "call statements" - VBA doesn't actually require a formal CALL statement, and if you DO use one, you have to remember parentheses. But from a syntax viewpoint, the following two statements are identical:

CALL MySub( x, y, z )
Mysub x, y, z

 

[RonPaii]

Each subroutine should do one thing. If you find yourself doing "exit sub" or deep" if then elseif", it's time to break it up. It also simplifies your error trapping with fewer errors to handle per routine.

 

[tvanstiphout]

If you have a DoEverything procedure, you probably also have procedure arguments that drive how the proc runs. E.g.
public sub DoEverything(arg1 as string, arg2 as long)
if arg1 = "A" then
'Code to do Thing1 goes here.
elseif arg1 = "B" and arg2=99 then
'Code to do Thing2 goes here

This is a clear example of where breaking things up would be beneficial for readability and testing and maintenance.
Note that even if you don't have arguments, getting these "driver variables" at the top of the proc is equivalent.

 

[smiler44]

Thank you all for the advice. I think I will add a couple of new modules, so that the main macro does not get diluted with code and I lose track of what code is proper to the task in hand.

thanks again
smiler44

 

[Isaac]

The general rule in ANY part of programming, whether we are talking databases, complex web sites, or some utility program, is "Divide and Conquer." There is a point where dividing up a subroutine goes too far, but from a pragmatist's point of view, "putting all the eggs in one basket" isn't such a good idea either.

A question you must ask is "can I use some part of this routine in more than one context?" Code-reuse is always a good thing if it is possible.

Some folks have a rule of thumb that says, if my routine's code won't fit on one screen, I have to split it into page-sized units. Other folks put limits like "no more than 100 lines". These are all self-imposed standards, or perhaps simply guidelines. You do what you have to do - but look for cases where you can isolate some routine to a very simple sequence, because that makes debugging easier when you can isolate code parts to become separate routines. Ease of debugging is NOT a trivial reason to split code into distinct parts.

As to "call statements" - VBA doesn't actually require a formal CALL statement, and if you DO use one, you have to remember parentheses. But from a syntax viewpoint, the following two statements are identical:

CALL MySub( x, y, z )
Mysub x, y, z

I like this response, because, while I agree in theory with most of what MajP said, yet, I do think there is a 'principle' that could be articulated (although I cannot articulate it) that governs when you separate things out. Just like Table design, it's not just a feeling or an instinct, it's principles that have to do with entities and relationships and those principles hard govern.

I'd say for me it's easier to say what that principle ISN'T. it isn't anythign that has to do with size or fitting on a screen. It's what code is going to call what other code. Another red flag is when you find yourself including LOTS of parameters in a single procedure. It's likely that there is a little-guy-procedure just waiting to be written to be the go between. Make each procedure do the most discreet thing it can or ought to do. There's, maybe, another one. If you violate this heartily, you'll just find yourself writing duplicate versions of the same procedure because you realize there is a variability in "all those things" that one procedure was doing. But of course: With a higher count of achievements the procedure has, the higher the variability and the more you'd wish you'd encapsulated in a smaller way.

If you move forward and write a bunch of complex procedures that do a lot of things, give it 6 mo. of usage........Your own requirements of how you had to constantly edit or duplicate those will teach you exactly what you need to know

 

[Isaac]

chatgpt gave me a few things as food for thought, and i'll repeat the ones i thought were good below.

Level of Abstraction Consistency​

Within a single procedure, all lines of code should operate at roughly the same conceptual level.
If one line reads input, another manipulates low-level data structures, and a third performs high-level business logic — you’ve mixed abstraction layers and should separate them.

If you can give part of your code a meaningful name that describes what it does (without reference to how it does it), it probably deserves to become its own procedure.​

Change Isolation​

Ask yourself: if one small requirement changes, will I need to edit this procedure in multiple spots?
If yes, it’s probably mixing responsibilities that should be isolated.
Good separation reduces the “blast radius” of change.

Guiding Heuristic​

“A procedure should express one complete thought at one level of abstraction.”

That’s the distilled principle many experienced coders subconsciously use — the same intuition you described.

Sorry for the obnoxious bold formatting - chat copy/paste

 

[The_Doc_Man]

Isaac, those are good points. Here's the old man's way of saying it.

Level of abstraction consistency = "purity of purpose" - Ideally, a routine should do only one thing; it has only one purpose. With perfect purity, you can read the code and if you know what each routine does in overview, you can quickly summarize everything that happens on simple reading.

Meaningful name = a side effect of purity of purpose, because if you CAN issue a meaningful name, it implies that the purpose should be pretty clear.

Change isolation = too many distributed changes means improper abstraction, because what should be a single place to do something... isn't. Which implies "impurity of purpose" OR "impurity of application of purpose."

Guiding Heuristic = "purity of purpose" to have a single thought at a single layer of abstraction.