Example of refactorization

To understand Peoplecode is one thing, to develop correctly using Peopletools is an other game.

I know sometimes it is easier to copy-paste multiple time the same piece of code, or to duplicate 10 times the same fields in a record, I did that in the past, when starting coding.  However, it is not the best practice at my eyes : the code is hard to maintain, because it is very long, but also because when a correction must be done, it must be applied for each copy of the code.

In my example, a table was built to store student insurance data for each semester; basically the structure of the table looks like :

EMPLID

STRM

EFFDT

EFFSEQ

FIELD1_SEP

FIELD2_SEP

FIELD3_SEP

...

FIELD1_AUG

FIELD2_AUG

FIELD3_AUG

First Problem : To repeat 12 times the same 3 fields in the same record is not appropriate, especially in this case, where some fields were used for a semester only (Ex : for Fall : SEP, OCT, NOV, DEC ).

To solve this problem, I created a child table : 

EMPLID

STRM

EFFDT

EFFSEQ

EFFSEQ2

MONTH

FIELD1

FIELD2

FIELD3

---

Now, lets jump to the code (second problem).  In fact, this was the first problem in time, I solve it, after, when I realized I couldn't work with the former record, I created the new child record and replace completely the code.

So, the code I had to read, decode in my head, understand, and modify, was about 800 lines long.  It was an Application Engine Peoplecode.  I started looking at the code, and quickly understood that it was made of the same block copied and pasted a lot of times, with minor differences; actually, I found out that there were two different blocks, repeated respectively 12 and 32 times.

Block1	For all months from SEP to currmonth - 1 -> 32 times	If STATERECORD_AET.FIELD1_NOV <> "X" Then          If STATERECORD_AET.UM_MONTH_NOV = "Y" Then             If STATERECORD_AET.FIELD3_NOV <> 0 Then                STATERECORD_AET.FIELD2_NOV = ( - STATERECORD_AET.FIELD3_NOV);             Else                STATERECORD_AET.FIELD2_NOV = ( - &MTTRI);             End-If;          Else             STATERECORD_AET.FIELD2_NOV = &MTTRI;          End-If;                  STATERECORD_AET.FIELD1_NOV = "X";       End-If;
Block2	1 time for each month -> 12 times	If STATERECORD_AET.FIELD1_DEC <> "X" Then          If STATERECORD_AET.UM_MONTH_DEC = "Y" Then             STATERECORD_AET.FIELD2_DEC = ( - STATERECORD_AET.FIELD3_DEC) + (&MTTRI);          Else             STATERECORD_AET.FIELD2_DEC = &MTTRI;          End-If;          STATERECORD_AET.FIELD1_DEC = "X";       End-If;

The code was organized like this :

IF strm = fall THEN
   IF month = 04 (December) THEN
        Part1 for September
        Part1 for October
        Part1 for November
        Part2 for December
  END-IF;
   IF month = 03 (November) THEN
        Part1 for September
        Part1 for October
        Part2 for November
  END-IF;

And so on, for each semester, for each month.  I can't imagine that someone copied the same code 32 times and replaced the suffix of each field... any way.

There was no way I would repeate a single change 12, 32 or 44 times.  So I took few hours to see if I could replace all this copy-past by the two parts only.  In fact, the only difference between the same part copied for two different months was the suffix corresponding to the month.  I remember that we can use the @ character to get an object (a field, a record) from a string containing its name, such as :

&rec1.GetField(@("FIELD." | &fieldname));

So I copied each block ONLY 1 MORE TIME, replaced the field name by the dynamic way to retrieve it, and applied the logic to apply each part the good number of times for each month.

Here is the result :

/*The months suffixes ; one field had suffixes in French and others had suffixes in English*/
&FieldSuffixes = CreateArray("SEP", "OCT", "NOV", "DEC", "JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG");
&AMTSuffixes = CreateArray("SEP", "OCT", "NOV", "DEC", "JAN", "FEV", "MAR", "AVR", "MAY", "JUN", "JUL", "AUG");

&MonthNumber = 0;

/*BLOCK1 being repeated for each month before the current month*/
For &MonthNumber = 1 To Value(APPENGINE_AET.CURRMONTH.Value) - 1
   &currFieldSuffix = &FieldSuffixes.Get(&MonthNumber);
   &currAmtFieldSuffix = &AMTSuffixes.Get(&MonthNumber);
   
   &currMthReported = &rec1.GetField(@("FIELD.FIELD1_" | &currFieldSuffix)).Value;
   &currMthMonth = &rec1.GetField(@("FIELD.FIELD2_" | &currFieldSuffix)).Value;
   &currMthAmount = &rec1.GetField(@("FIELD.FIELD3_" | &currFieldSuffix)).Value;
   
   /*BLOCK1*/
   If &currMthReported <> "X" Then
      If &currMthMonth = "Y" Then
         If &existsLowerRow = "X" Then 
            If &currMthAmount <> 0 Then
               &rec1.GetField(@("FIELD.FIELD3_" | &currAmtFieldSuffix )).Value = ( - &currMthAmount);
            Else
               &rec1.GetField(@("FIELD.FIELD3_" | &currAmtFieldSuffix )).Value = ( - &MTTRI);
            End-If;
         End-If; 
         
      Else
         &rec1.GetField(@("FIELD.FIELD3_" | &currAmtFieldSuffix )).Value = &MTTRI;
      End-If;
      
      &rec1.GetField(@("FIELD.FIELD1_" | &currFieldSuffix)).Value = "X";
   End-If;
   
   
End-For;

&currFieldSuffix = &FieldSuffixes.Get(Value(APPENGINE_AET.CURRMONTH.Value));
&currAmtFieldSuffix = &AMTSuffixes.Get(Value(APPENGINE_AET.CURRMONTH.Value));

&currMthReported = &rec1.GetField(@("FIELD.FIELD1_" | &currFieldSuffix)).Value;
&currMthMonth = &rec1.GetField(@("FIELD.FIELD2_" | &currFieldSuffix)).Value;
&currMthAmount = &rec1.GetField(@("FIELD.FIELD3_" | &currFieldSuffix)).Value;

/*BLOCK2*/
If &currMthReported <> "X" Then
   If &currMthMonth = "Y" Then
      &rec1.GetField(@("FIELD.FIELD3_" | &currAmtFieldSuffix)).Value = ( - &currMthAmount) + (&MTTRI);
      
   Else
      &rec1.GetField(@("FIELD.FIELD3_" | &currAmtFieldSuffix)).Value = &MTTRI;
   End-If;
   &rec1.GetField(@("FIELD.FIELD1_" | &currFieldSuffix)).Value = "X";
   
End-If;

I know this is not rocket science, but not everybody would have thought to use dynamic fields.  Some strong developper debugged this code before me but none of them had this idea.  This is why I share it, so next time you see an opportunity to reduce the code, you may be inspire and you know were to look for an example.