ABAP - Refactoring of legacy code (2)
This is about the second part of the refactoring series. We'll show you the final steps to convert the old coding into a new form.
Table of contents
In the last article we mainly created the basis for the new report. Today it is about the adjustments within the structure, the clearing up of the variables and the finalization for the test. As always, Eclipse is the greatest helper in restructuring the old code.
Phase: Adjustment
The adjustment phase is about the first major changes in the source code. Identified redundancies and global variables are now reduced. Further helper methods can be created and used, the code slowly gets its final structure.
Create methods
Next, additional methods are created to get redundancies from the coding and to simplify the structuring of the process. To do this, we create a method to validate the company code, since there are two identical accesses. The validation of the selection screen and the selection of the data are also packed into separate methods. With that, your code should have the following structure when completed:
REPORT ztest_determine_bookings.
TYPES:
BEGIN OF ts_booking,
bukrs TYPE bkpf-bukrs,
belnr TYPE bkpf-belnr,
gjahr TYPE bkpf-gjahr,
cpudt TYPE bkpf-cpudt,
xblnr TYPE bkpf-xblnr,
buzei TYPE bseg-buzei,
wrbtr TYPE bseg-wrbtr,
waers TYPE bkpf-waers,
kostl TYPE bseg-kostl,
prctr TYPE bseg-prctr,
butxt TYPE t001-butxt,
END OF ts_booking.
TYPES: tt_booking TYPE STANDARD TABLE OF ts_booking.
DATA:
gd_date TYPE d.
FIELD-SYMBOLS:
<gs_booking> TYPE ts_booking.
SELECTION-SCREEN BEGIN OF BLOCK b01 WITH FRAME TITLE TEXT-t01.
PARAMETERS:
p_bukrs TYPE bkpf-bukrs,
p_belnr TYPE bkpf-belnr,
p_gjahr TYPE bkpf-gjahr.
SELECT-OPTIONS:
s_date FOR gd_date.
SELECTION-SCREEN END OF BLOCK b01.
CLASS lcl_report DEFINITION FINAL.
PUBLIC SECTION.
METHODS:
initialization,
main,
on_bukrs.
PRIVATE SECTION.
DATA:
gt_data TYPE STANDARD TABLE OF t001,
gs_data TYPE t001,
gt_booking TYPE tt_booking,
gs_booking TYPE ts_booking,
go_my_table TYPE REF TO cl_salv_table.
METHODS:
output,
validate_parameters
RETURNING VALUE(rd_valid) TYPE abap_bool,
validate_company_code
IMPORTING
id_bukrs TYPE bkpf-bukrs
RETURNING VALUE(rd_valid) TYPE abap_bool,
selection.
ENDCLASS.
CLASS lcl_report IMPLEMENTATION.
METHOD initialization.
DATA ls_date LIKE LINE OF s_date.
ls_date-sign = 'I'.
ls_date-option = 'BT'.
ls_date-low = '20200101'.
ls_date-high = sy-datum.
APPEND ls_date TO s_date.
ENDMETHOD.
METHOD on_bukrs.
IF p_bukrs IS NOT INITIAL.
IF validate_company_code( p_bukrs ) = abap_false.
MESSAGE 'Company Code is not valid!' TYPE 'E'.
ENDIF.
ENDIF.
ENDMETHOD.
METHOD main.
IF validate_parameters( ) = abap_false.
RETURN.
ENDIF.
selection( ).
output( ).
ENDMETHOD.
METHOD validate_parameters.
IF p_bukrs IS INITIAL OR p_belnr IS INITIAL OR p_gjahr IS INITIAL.
MESSAGE 'Required fields not filled!' TYPE 'E'.
rd_valid = abap_false.
ELSE.
IF validate_company_code( p_bukrs ) = abap_false.
MESSAGE 'Company Code is not valid!' TYPE 'E'.
rd_valid = abap_false.
ELSE.
rd_valid = abap_true.
ENDIF.
ENDIF.
ENDMETHOD.
METHOD selection.
DATA:
ls_bkpf TYPE bkpf,
ls_bseg TYPE bseg.
SELECT * FROM bkpf INTO ls_bkpf
WHERE bukrs = p_bukrs
AND belnr = p_belnr
AND gjahr = p_gjahr.
SELECT * FROM bseg INTO ls_bseg
WHERE bukrs = p_bukrs
AND belnr = p_belnr
AND gjahr = p_gjahr.
CLEAR gs_booking.
MOVE-CORRESPONDING ls_bkpf TO gs_booking.
gs_booking-buzei = ls_bseg-buzei.
gs_booking-wrbtr = ls_bseg-wrbtr.
gs_booking-kostl = ls_bseg-kostl.
gs_booking-prctr = ls_bseg-prctr.
APPEND gs_booking TO gt_booking.
ENDSELECT.
ENDSELECT.
SELECT * FROM t001 INTO TABLE gt_data.
LOOP AT gt_booking ASSIGNING <gs_booking>.
READ TABLE gt_data INTO gs_data
WITH KEY bukrs = <gs_booking>-bukrs.
IF sy-subrc = 0.
<gs_booking>-butxt = gs_data-butxt.
ENDIF.
ENDLOOP.
ENDMETHOD.
METHOD output.
cl_salv_table=>factory(
IMPORTING
r_salv_table = go_my_table
CHANGING
t_table = gt_booking
).
go_my_table->display( ).
ENDMETHOD.
METHOD validate_company_code.
SELECT SINGLE * FROM t001 INTO gs_data WHERE bukrs = id_bukrs.
rd_valid = xsdbool( sy-subrc = 0 ).
ENDMETHOD.
ENDCLASS.
INITIALIZATION.
DATA(go_report) = NEW lcl_report( ).
go_report->initialization( ).
AT SELECTION-SCREEN ON p_bukrs.
go_report->on_bukrs( ).
START-OF-SELECTION.
go_report->main( ).
Global variables
In this step we try to remove or minimize the global variables. The best way to do this is to look at the last global variables and go through method by method to see which data are actually local. The parameters for the methods are also defined in this step.
At the end you can use the Eclipse function "Source Code -> Delete unused variables (all)" to remove all unused variables. You will quickly find that there are no more variables left and none are needed globally.
Rename
In this step, we name the methods a little more descriptive in order to understand what the report and the individual steps do now. You should also set everything here to the current naming conventions in order to pass your ATC check. The oldest language constructs should also have disappeared from the source code, i.e. at least what has already been marked as obsolete by SAP.
At the end of the phase, the source code should look something like this:
REPORT ztest_determine_bookings.
DATA:
gd_date TYPE d.
SELECTION-SCREEN BEGIN OF BLOCK b01 WITH FRAME TITLE TEXT-t01.
PARAMETERS:
p_bukrs TYPE bkpf-bukrs,
p_belnr TYPE bkpf-belnr,
p_gjahr TYPE bkpf-gjahr.
SELECT-OPTIONS:
s_date FOR gd_date.
SELECTION-SCREEN END OF BLOCK b01.
CLASS lcl_report DEFINITION FINAL.
PUBLIC SECTION.
TYPES:
BEGIN OF ts_booking,
bukrs TYPE bkpf-bukrs,
belnr TYPE bkpf-belnr,
gjahr TYPE bkpf-gjahr,
cpudt TYPE bkpf-cpudt,
xblnr TYPE bkpf-xblnr,
buzei TYPE bseg-buzei,
wrbtr TYPE bseg-wrbtr,
waers TYPE bkpf-waers,
kostl TYPE bseg-kostl,
prctr TYPE bseg-prctr,
butxt TYPE t001-butxt,
END OF ts_booking,
tt_booking TYPE STANDARD TABLE OF ts_booking WITH DEFAULT KEY.
METHODS:
initialization,
main,
on_bukrs.
PRIVATE SECTION.
METHODS:
output_booking
IMPORTING
it_booking TYPE tt_booking,
validate_parameters
RETURNING VALUE(rd_valid) TYPE abap_bool,
validate_company_code
IMPORTING
id_bukrs TYPE bkpf-bukrs
RETURNING VALUE(rd_valid) TYPE abap_bool,
selection
RETURNING VALUE(rt_booking) TYPE tt_booking.
ENDCLASS.
CLASS lcl_report IMPLEMENTATION.
METHOD initialization.
DATA:
ls_date LIKE LINE OF s_date.
ls_date-sign = 'I'.
ls_date-option = 'BT'.
ls_date-low = '20200101'.
ls_date-high = sy-datum.
APPEND ls_date TO s_date.
ENDMETHOD.
METHOD on_bukrs.
IF p_bukrs IS NOT INITIAL AND validate_company_code( p_bukrs ) = abap_false.
MESSAGE 'Company Code is not valid!' TYPE 'E'.
ENDIF.
ENDMETHOD.
METHOD main.
IF validate_parameters( ) = abap_false.
RETURN.
ENDIF.
DATA(lt_booking) = selection( ).
output_booking( lt_booking ).
ENDMETHOD.
METHOD validate_parameters.
IF p_bukrs IS INITIAL OR p_belnr IS INITIAL OR p_gjahr IS INITIAL.
MESSAGE 'Required fields not filled!' TYPE 'E'.
rd_valid = abap_false.
ELSE.
IF validate_company_code( p_bukrs ) = abap_false.
MESSAGE 'Company Code is not valid!' TYPE 'E'.
rd_valid = abap_false.
ELSE.
rd_valid = abap_true.
ENDIF.
ENDIF.
ENDMETHOD.
METHOD selection.
DATA:
ls_booking TYPE ts_booking,
ls_bkpf TYPE bkpf,
ls_bseg TYPE bseg.
SELECT * FROM bkpf INTO ls_bkpf
WHERE bukrs = p_bukrs
AND belnr = p_belnr
AND gjahr = p_gjahr.
SELECT * FROM bseg INTO ls_bseg
WHERE bukrs = p_bukrs
AND belnr = p_belnr
AND gjahr = p_gjahr.
CLEAR ls_booking.
MOVE-CORRESPONDING ls_bkpf TO ls_booking.
ls_booking-buzei = ls_bseg-buzei.
ls_booking-wrbtr = ls_bseg-wrbtr.
ls_booking-kostl = ls_bseg-kostl.
ls_booking-prctr = ls_bseg-prctr.
APPEND ls_booking TO rt_booking.
ENDSELECT.
ENDSELECT.
SELECT * FROM t001 INTO TABLE @DATA(lt_company_codes).
LOOP AT rt_booking ASSIGNING FIELD-SYMBOL(<ls_booking>).
READ TABLE lt_company_codes INTO DATA(ls_company_code)
WITH KEY bukrs = <ls_booking>-bukrs.
IF sy-subrc = 0.
<ls_booking>-butxt = ls_company_code-butxt.
ENDIF.
ENDLOOP.
ENDMETHOD.
METHOD output_booking.
DATA(lt_booking) = it_booking.
cl_salv_table=>factory(
IMPORTING
r_salv_table = DATA(lo_my_table)
CHANGING
t_table = lt_booking
).
lo_my_table->display( ).
ENDMETHOD.
METHOD validate_company_code.
SELECT SINGLE * FROM t001 INTO @DATA(ls_data) WHERE bukrs = @id_bukrs.
rd_valid = xsdbool( sy-subrc = 0 ).
ENDMETHOD.
ENDCLASS.
INITIALIZATION.
DATA(go_report) = NEW lcl_report( ).
go_report->initialization( ).
AT SELECTION-SCREEN ON p_bukrs.
go_report->on_bukrs( ).
START-OF-SELECTION.
go_report->main( ).
Phase: Finalize
The final phase is all about final adjustments, performance and code optimizations to make the final report "round". Here you could e.g. also look at the selects and optimize something. Furthermore, the report should be translateable and testable.
Format
In this step the methods and function module calls as well as the selects are brought into a properly formatted state. The field lists of the selects can be reviewed, especially when working with the wildcard *, to see whether all fields are really required.
Text
Now the task is to outsource all texts to text symbols so that the report can be translated. Furthermore, messages should also be outsourced to their own message classes so that they can also be translated properly.
Even if your system currently only supports one language, multilingualism may become an issue in the future. In this case, all of your objects would be prepared for this case.
Test
Finally, the most important thing, the actual functionality test. Is the report still behaving as it should? Here the report and its functions have to be fully checked and tested again. In most cases it is also worth writing a unit test and thus testing the functions automatically.
Result
Here again the final result of the report after taking over and adjusting the source code. This is now easier and cheaper to maintain and is easier to understand for any developer. During the optimization, we implemented a few points from the Clean Code for ABAP concept in order to make the source code even tidier.
REPORT ztest_determine_bookings.
DATA:
gd_date TYPE d.
SELECTION-SCREEN BEGIN OF BLOCK b01 WITH FRAME TITLE TEXT-t01.
PARAMETERS:
p_bukrs TYPE bkpf-bukrs,
p_belnr TYPE bkpf-belnr,
p_gjahr TYPE bkpf-gjahr.
SELECT-OPTIONS:
s_date FOR gd_date.
SELECTION-SCREEN END OF BLOCK b01.
CLASS lcl_report DEFINITION FINAL.
PUBLIC SECTION.
TYPES:
BEGIN OF ts_booking,
bukrs TYPE bkpf-bukrs,
belnr TYPE bkpf-belnr,
gjahr TYPE bkpf-gjahr,
cpudt TYPE bkpf-cpudt,
xblnr TYPE bkpf-xblnr,
buzei TYPE bseg-buzei,
wrbtr TYPE bseg-wrbtr,
waers TYPE bkpf-waers,
kostl TYPE bseg-kostl,
prctr TYPE bseg-prctr,
butxt TYPE t001-butxt,
END OF ts_booking,
tt_booking TYPE STANDARD TABLE OF ts_booking WITH DEFAULT KEY.
METHODS:
initialization,
main,
on_bukrs.
PRIVATE SECTION.
METHODS:
output_booking
IMPORTING
it_booking TYPE tt_booking,
validate_parameters
RETURNING VALUE(rd_valid) TYPE abap_bool,
validate_company_code
IMPORTING
id_bukrs TYPE bkpf-bukrs
RETURNING VALUE(rd_valid) TYPE abap_bool,
selection
RETURNING VALUE(rt_booking) TYPE tt_booking.
ENDCLASS.
CLASS lcl_report IMPLEMENTATION.
METHOD initialization.
INSERT VALUE #( sign = 'I' option = 'BT' low = '20200101' high = sy-datum ) INTO TABLE s_date[].
ENDMETHOD.
METHOD on_bukrs.
IF p_bukrs IS NOT INITIAL AND validate_company_code( p_bukrs ) = abap_false.
MESSAGE TEXT-001 TYPE 'E'.
ENDIF.
ENDMETHOD.
METHOD main.
IF validate_parameters( ) = abap_false.
RETURN.
ENDIF.
DATA(lt_booking) = selection( ).
output_booking( lt_booking ).
ENDMETHOD.
METHOD validate_parameters.
IF p_bukrs IS INITIAL OR p_belnr IS INITIAL OR p_gjahr IS INITIAL.
MESSAGE TEXT-002 TYPE 'E'.
rd_valid = abap_false.
ELSE.
IF validate_company_code( p_bukrs ) = abap_false.
MESSAGE TEXT-001 TYPE 'E'.
rd_valid = abap_false.
ELSE.
rd_valid = abap_true.
ENDIF.
ENDIF.
ENDMETHOD.
METHOD selection.
SELECT bukrs, belnr, gjahr, cpudt, xblnr, waers
FROM bkpf
WHERE bukrs = @p_bukrs
AND belnr = @p_belnr
AND gjahr = @p_gjahr
INTO TABLE @DATA(lt_bkpf).
LOOP AT lt_bkpf INTO DATA(ls_bkpf).
SELECT buzei, wrbtr, kostl, prctr
FROM bseg
WHERE bukrs = @p_bukrs
AND belnr = @p_belnr
AND gjahr = @p_gjahr
INTO TABLE @DATA(lt_bseg).
LOOP AT lt_bseg INTO DATA(ls_bseg).
DATA(ls_booking) = CORRESPONDING ts_booking( ls_bkpf ).
ls_booking = CORRESPONDING #( BASE ( ls_booking ) ls_bseg ).
INSERT ls_booking INTO TABLE rt_booking.
ENDLOOP.
ENDLOOP.
LOOP AT rt_booking ASSIGNING FIELD-SYMBOL(<ls_booking>).
SELECT SINGLE butxt
FROM t001
WHERE bukrs = @<ls_booking>-bukrs
INTO @<ls_booking>-butxt.
ENDLOOP.
ENDMETHOD.
METHOD output_booking.
DATA(lt_booking) = it_booking.
cl_salv_table=>factory(
IMPORTING
r_salv_table = DATA(lo_my_table)
CHANGING
t_table = lt_booking
).
lo_my_table->display( ).
ENDMETHOD.
METHOD validate_company_code.
SELECT SINGLE * FROM t001 INTO @DATA(ls_data) WHERE bukrs = @id_bukrs.
rd_valid = xsdbool( sy-subrc = 0 ).
ENDMETHOD.
ENDCLASS.
INITIALIZATION.
DATA(go_report) = NEW lcl_report( ).
go_report->initialization( ).
AT SELECTION-SCREEN ON p_bukrs.
go_report->on_bukrs( ).
START-OF-SELECTION.
go_report->main( ).
Conclusion
With our little help, refactoring or migrating reports should no longer be a problem for you. Step by step you are now getting a little closer to the goal and after each phase you have a consistent result that you can improve further. In the end, you should attach great importance to clean code so that the report is future-proof.
