RPG Subroutine - want to confirm it is fine.

In this subroutine, we want to see which Customer address records, are not in the Order history,
and if not in there, we want to see if not in the Invoice history. In which cases we want to purge this customer. We want to write each record prior to the delete. We do not want to check the same customer twice +. as there are duplicate Customer numbers (entity #). Is this Subroutine have any holes you might notice?



C     CHKORH_SR     BEGSR                                                    
 *                                                                            
C                   MOVE      'N'           PUGFIL            1              
C     ORHKEY        CHAIN     OEORH4                                          
 * If the order entity is notfound, write the rec into TRCMASAC file          
C                   IF        NOT %FOUND(OEORH4)                              
C                   MOVE      'Y'           PUGFIL                            
C                   endif                                                    
c     pugfil        ifne      'Y'                                            
C     ORHKEY        CHAIN     OEinh4                                          
C                   IF        NOT %FOUND(OEinh4)                              
C                   MOVE      'Y'           PUGFIL                            
C                   ENDIF                                                    
C                   ENDIF                                                    
 *                                                                            
C                   IF        PUGFIL = 'Y' AND        
C                             ACENT# <> ACENT#_OLD                      
c                   EXSR      CHKCUS_SR                                
c     ACFLAG        IFEQ      'N'                                      
C                   WRITE     TRCMASRR                                  
c                   delete    arcmasrr                                  
 * Check the record does not exist in stock header file                
 *                                                                      
c     acflag        ifeq      'N'                                      
C                   EXSR      CHKADR_SR                                
c*                                                                      
c                   ENDIF                                              
C                   ENDIF                                              
C                   ENDIF                                              
C                   MOVE      ACENT#        ACENT#_OLD                  
 *                                                                      
C                   ENDSR
Philky101Asked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Gary PattersonVP Technology / Senior Consultant Commented:
Functionally, it looks OK to me.  No way to tell for sure without testing.  

I don't understand what this means:

We do not want to check the same customer twice +. as there are duplicate Customer numbers (entity #).


But if all you need to do is check if the current key doesn't exist in the two files mentioned, then the logic looks OK.

Would be more efficient if you eliminated the unneeded IF test, and just used and ELSE clause:

pugfil        ifne      'Y'    

And if you replaced the CHAINs with SETLLs.  SETLL is much faster and more efficient if all you need to do is check to see if a record exists, since it doesn't actually read the record - it just checks the index:

C     CHKORH_SR     BEGSR                                                    
 *                                                                            
C                   MOVE      'N'           PUGFIL            1              
C     ORHKEY        SETLL     OEORH4                                          
 * If the order entity is notfound, write the rec into TRCMASAC file          
C                   IF        NOT %EQUAL(OEORH4)                              
C                   MOVE      'Y'           PUGFIL                            
C                   ELSE                                                    
C     ORHKEY        SETLL     OEinh4                                          
C                   IF        NOT %EQUAL(OEinh4)                              
C                   MOVE      'Y'           PUGFIL                            
C                   ENDIF                                                    
C                   ENDIF                                                    
 *                                                                            
C                   IF        PUGFIL = 'Y' AND        
C                             ACENT# <> ACENT#_OLD                      
c                   EXSR      CHKCUS_SR                                
 ***CHKCUS_SR sets ACFLAG?
c     ACFLAG        IFEQ      'N'                                      
C                   WRITE     TRCMASRR                                  
c                   delete    arcmasrr                                  
 * Check the record does not exist in stock header file                
 *                                                                      
c     acflag        ifeq      'N'                                      
C                   EXSR      CHKADR_SR                                
c*                                                                      
c                   ENDIF                                              
C                   ENDIF                                              
C                   ENDIF                                              
C                   MOVE      ACENT#        ACENT#_OLD                  
 *                                                                      
C                   ENDSR 

Open in new window


Of course, no matter what you do, you need to test carefully.  I have no idea what the data looks like on your system.
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
tliottaCommented:
As posted in the question, it looks incorrect to me. But it's not at all clear what "correct" code should look like.

If a matching record exists in OEORH4, then PUGFIL will keep the value 'N'. But then comes:
c     pugfil        ifne      'Y'

Open in new window

That is, the test for existence in OEinh4 only happens if a record does exist in OEORH4. If a record isn't then found in OEinh4, PUGFIL is finally set to 'Y'.

So the only way the DELETE can happen is if a matching record is found in OEORH4 and not found in OEinh4, plus the added tests for ACENT# <> ACENT#_OLD and ACFLAG = 'N'.

That doesn't seem to match the description of the problem where neither file should have a matching record.

In order to know what any correct coding should look like, the problem needs to be stated much more clearly.

Tom
0
Gary PattersonVP Technology / Senior Consultant Commented:
Tom, as usual , raises a good point - though the current code will delete in two cases:

1) Delete if not found in OEORH4 (OEINH4 won't be checked)
2) Delete if found in OEORH4, but not found in OEINH4

Or stated as a single rule: "Delete if missing in either file."  

If the rule is "Delete if missing in BOTH files" then you want to do something like this:

C     CHKORH_SR     BEGSR                                                    
 *                                                                            
C                   MOVE      'N'           PUGFIL            1              
C     ORHKEY        SETLL     OEORH4                                          
 * If the order entity is notfound, write the rec into TRCMASAC file          
C                   IF        NOT %EQUAL(OEORH4)                              
C     ORHKEY        SETLL     OEinh4                                          
C                   IF        NOT %EQUAL(OEinh4)                              
C                   MOVE      'Y'           PUGFIL                            
C                   ENDIF                                                    
C                   ENDIF                                                    
 *                                                                            
C                   IF        PUGFIL = 'Y' AND        
C                             ACENT# <> ACENT#_OLD                      
c                   EXSR      CHKCUS_SR                                
 ***CHKCUS_SR sets ACFLAG?
c     ACFLAG        IFEQ      'N'                                      
C                   WRITE     TRCMASRR                                  
c                   delete    arcmasrr                                  
 * Check the record does not exist in stock header file                
 *                                                                      
c     acflag        ifeq      'N'                                      
C                   EXSR      CHKADR_SR                                
c*                                                                      
c                   ENDIF                                              
C                   ENDIF                                              
C                   ENDIF                                              
C                   MOVE      ACENT#        ACENT#_OLD                  
 *                                                                      
C                   ENDSR 

Open in new window

0
The Ultimate Tool Kit for Technolgy Solution Provi

Broken down into practical pointers and step-by-step instructions, the IT Service Excellence Tool Kit delivers expert advice for technology solution providers. Get your free copy for valuable how-to assets including sample agreements, checklists, flowcharts, and more!

tliottaCommented:
Heh, yep, no check at all for OEinh4 when not found in OEORH4. I should've looked deeper, but the question was already closed and I only wanted to get attention back to it.

The SETLLs and NOT %EQUAL() tests are the way to go, though it's not clear where the ACENT# value comes from. We have to assume that its set before the subroutine is called. If none of the field values from OEORH4 and OEinh4 are used by the program, the files should not be CHAINed to. Just use SETLL to do an existence test.

Once SETLL runs, there's no need for setting PUGFIL. It can be replaced by [ NOT (%EQUAL(OEORH4) OR %EQUAL(OEinh4)) ]. Even that doesn't seem need as long as the WRITE and DELETE are inside the IF-tests anyway.

If I understand the actual requirement (which isn't certain), the logic would seem like this:
C     CHKORH_SR     BEGSR

C                   IF        ACENT# <> ACENT#_OLD

C     ORHKEY        SETLL     OEORH4
 * If the order entity is notfound, write the rec into TRCMASAC file
C                   IF        NOT %EQUAL(OEORH4)

C     ORHKEY        SETLL     OEinh4
C                   IF        NOT %EQUAL(OEinh4)
c                   EXSR      CHKCUS_SR

 ***CHKCUS_SR sets ACFLAG?
c                   IF        ACFLAG = 'N'

C                   WRITE     TRCMASRR
c                   delete    arcmasrr

 * Check the record does not exist in stock header file
C                   EXSR      CHKADR_SR

C                   ENDIF
C                   ENDIF
C                   ENDIF

C                   MOVE      ACENT#        ACENT#_OLD

C                   ENDIF

C                   ENDSR

Open in new window

That seems to have the same logical result as the original coding while meeting requirements. Since the original contains %FOUND(), etc., we can assume RPG IV. In that case, I'd probably do it a little differently. But that gets into coding styles rather than trying to determine what logic is correct.

Note that it's not clear why CHKADR_SR should be executed in this sequence. It doesn't seem to have a purpose inside this subroutine.

Tom
0
Gary PattersonVP Technology / Senior Consultant Commented:
Yeah - I just ignored the parts that weren't covered in the code shown - too much conjecture required to suit me, and focused on the parts mentioned in the question - just the "top part".  

I thought there was a chance, based on prefix and location in the code, that ACFLAG and ACENT# might be updated in CHKCUS_SR, so I didn't touch any of that.
0
tliottaCommented:
The results shouldn't change for ACFLAG and ACENT# by the logic rearrangement.

There wasn't any point in having a test for [ACFLAG = 'N'] nested inside a test for [ACFLAG = 'N'] when there were no intervening statements that could modify the value.

The test for [ACENT# <> ACENT#_OLD] might as well be the outer test after the CHAINs become SETLLs; the CHAINs were the only possible modifiers. And the only reason to do the MOVE for ACENT# would be if [ACENT# <> ACENT#_OLD]. Might as well enclose it, too. And since it's enclosed along with the call to CHKCUS_SR, any change there would still be caught.

Now, if the CHAINs really are needed...?

Tom
0
Gary PattersonVP Technology / Senior Consultant Commented:
I see what you mean - second ACFLAG check is redundant.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
DB2

From novice to tech pro — start learning today.