ibm midrange - looking for sharper way to code this RPG LE -

--- here have located customer numbers want purge in other files. first reading customer master, see if customer number exists, in order history or invoice history. if not, want purge customer customer master 2 other files.

however in second file, if customer number has 'a' or 'c' in marketing column, , it's after 2007, don't want purge 1 of files.

so made code before writes customer record save/hold file , deletes, gets flag, yes, ok remove.

c                   if        pugfil = 'y' ,         c                             acent# <> acent#_old     c                   exsr      chkcus_sr                c     acflag        ifeq      'n'                      c                   write     trcmasrr                 c*                  delete    arcmasrr                 

---

c     chkcus_sr     begsr       c                   eval      acflag = ' '                         c     orhkey        setll     drcst1                               c     orhkey        reade     drcst1                                * if order entity found, write rec vrcstkbi file  c                   dow       not %eof(drcst1)                     c                   if        bicotc <> 'a' , bicotc <> 'c'      c                   write     vrcstkrr                             c                   eval      acflag = 'n'                         c                   endif                                          c                   if        bicotc = 'a'                         c                   if        bistpd <  20070101                   c                   write     vrcstkrr                             c                   eval      acflag = 'n'                         c                   endif                                          c                   endif                                          c                   if        bicotc = 'c'                         c                   if        bistpd <  20070101                   c                   write     vrcstkrr                             c                   eval      acflag = 'n'          c                   endif                           c                   endif                           c     acflag        ifeq      'n'                   c                   exsr      chkadr_sr             

i'd write function (sub-procedure). local variables make code easier work because don't collide variables in mainline. find creating function helps me organise thoughts, , organised thoughts write better code. 'better' of course subjective, here mean easier understand , importantly, easier change without breaking else in process.

the variable names... ooh. use longer names - names make sense. acflag make life worse next time have @ (maybe in year? 7 years?) prefer benny's do_purge - says intends. have been indicator variable hammer home point it's yes/no decision point, it's easier understand if do_purge = 'y'than understand if acflag = 'n'. negative logic adds problem. subroutine suffers same cryptic naming convention. check customer. check what? what's business functionality being implemented? if can't described name, it's complex - doing many things. business function 'check active customer'? name subroutine way (or better still, write function name way). mainline become

if custisinactive(customerid);   exsr purge_customer; endif; 

comments. benny did job had work with. original code has 1 comment, , it's unhelpful. can see order entity found - that's if not %eof() means. , can see we're going write record. there's no explanation of why these actions important, desirable , useful. here's helps me lot. write comments first. not pseudocode! worst comments in history of universe this:

// if x > 10, reset y if x > 10;   y = 0; endif; 

that comment distracts eye code. white space better. simple code requires no comment. more complex code benefits comment explains intent. sort of comments code? in english, explain why codes , c important. maybe it's because

// don't purge customers we've done business // exception: if emailed or cold called them //            , didn't buy in  //            past 6 years, purge them. if (bicotc = 'a' , bistpd >= 20070101) or    (bicotc = 'c' , bistpd >= 20070101);   do_purge = 'y'; endif; 

i realise posted code doesn't this, side of glass can't tell if intended way it's written, or it's bug haven't tripped yet. comments should clarify intent. believe me, when next person gets code, he'll happy read plain english reason codes , c, , why specific date important. maybe it's date of merger or acquisition, , , c items came old division... code as-is doesn't explain.

if comments frowned upon (and in places) @ least avoid 'magic codes' , 'magic numbers'. how this:

if (bicotc = ohio_big_ticket , bistpd >= merger_date) or    (bicotc = ohio_little_ticket , bistpd >= merger_date);   do_purge = 'y'; endif; 

finally, concept of doing 1 thing @ time. 'check customer' subroutine more 'check' - it's writing records vrcstkbi. looks bug me based on description of situation. based on setll/reade loop, appears code looking through order history file. subroutine write vrcstkbi first 10 items , 11th makes customer ineligible purge. there records in vrcstkbi customer. whoops. often, we're tempted bundle multiple i/o operations in name of efficiency. i'm here tell after 35 years of doing this, agree donald knuth:

"we should forget small efficiencies, 97% of time: premature optimization root of evil."

think business functions.

1. is customer eligible purged?
2. record customer purged.
3. purge customer daughter files.

if have separate functions each business operation easier write, understand test, debug , modify in future. write 3 sub-procedures, name them names , deploy them in mainline:

if custisinactive(customerid);   record_purged_customerid(customerid);   purge_customer(customerid); endif; 

a function either returns information (custisinactive) or provides service (purge_customer). there shouldn't business logic making decisions in 'provide service' functions, , function that's serving information shouldn't implementing services. not because mix , match inherently evil, because more things slice of code does, harder understand , maintain. can keep small number of things in our active memory, ability abstract out business function single item - function name - powerful aid making robust programs.