Re: RPG Refactoring - removing GOTO - tips & techniques?

On 10/12/2017 1:43 PM, Charles Wilt wrote:

Looking at an old program that using GOTO...I'd like to get rid of it.

---Option 1---

dou *on = *on; //run once
//mainline part1
if something leave
//mainline part2
if somethingelse leave
//mainline part 3
enddo;
eval parm12 = *IN12;
return;

I've seen a lot of code. I'm not keen on GOTO myself, but this kind
(early exit) is the least troublesome for my brain. I'm not one to
argue that LEAVE is GOTO with an accent; it's a forward-only jump to a
single destination, so it's pretty easy to grok the intent.

This pattern is nice because when I go to add a new condition, the LEAVE
is automatic in my head due to the DO. So when I'm maintaining this a
year from now I'm unlikely to break it by adding new code. The downside
is that the DO triggers an expectation of a loop.

---Option 2---

//mainline part1
if something exsr @END_PROGRAM
//mainline part2
if somethingelse exsr @END_PROGRAM
//mainline part 3
...
exsr @END_PROGRAM
//shouldn't get here, but to be safe
return;

begsr @END_PROGRAM;
eval parm12 = *IN12;
return;
endsr;

Although I've done this, I don't care for it. EXSR is hard-wired in my
head that the code will go there _and come back_ -- but it never does in
this case. The name @END_PROGRAM is a good hint about that though :-)

Some of the programs I work on are monstrous in terms of lines of code.
So I might not readily see that early exits are happening via EXSR and I
might be tempted to put a RETURN in there which would break the code.

There is another pattern which you might consider:

success1 = mainline_part1(parm: parm: parm);
if success1;
success2 = mainline_part2(parm: parm: parm);
if success2;
success3 = mainline_part3(parm: parm: parm);
...
else;
// report the issue
endif;
else;
// report the issue
endif;

Once the mainline starts being refactored into sub-procedures, the
individual functions within each part can start getting refactored as
well. This has lead me to code which is easier and easier to understand
and maintain as time has passed. Local variables are heaven-sent!

Eventually I ended up with:
passes_credit_check = check_credit(parm:...
if passes_credit_check;
all_in_stock = reserve_items(parm:...
if all_in_stock;
prerequisites_met = check_customer_prereqs(parm:...
if prerequisites_met;
order_placed = place_order(parm:...
if not order_placed;
// report reason

All of that used to be mainline_part1...