In this blog-post i want to show how to refactor ABAP source-code using sensing variables.
Sensing variables were a idea invented by Micheal Feathers, which help to preserve existing behavior after refactorings. Refactoring in this context means to split a long procedure into smaller procedures.
In this blog-post i want to modify Feathers idea a bit, so that it is more suitable for ABAP developers.
Let’s show an example. The following messy procedure searches flights in a period of time from city A to city B, uses the function-module ZFLIGHT_PRICE_CALCULATION to calculate a up-to-date price and let the user choose one flight:
FORM flight_search USING city_to TYPE s_to_city city_from TYPE s_from_cit
begin_date TYPE d end_date TYPE d
CHANGING choosen_flight TYPE sflight.
DATA: schedule TYPE STANDARD TABLE OF spfli,
available_flights TYPE flighttab,
flight TYPE sflight.
SELECT * FROM spfli INTO TABLE schedule
WHERE cityto = city_to AND cityfrom = city_from.
SELECT * FROM sflight INTO TABLE available_flights
FOR ALL ENTRIES IN schedule
WHERE carrid = schedule-carrid AND connid = schedule-connid
AND fldate BETWEEN begin_date AND end_date.
" price is calculated again based on availability
CALL FUNCTION 'ZFLIGHT_PRICE_CALCULATION'
TABLES
flights = available_flights.
LOOP AT available_flights INTO flight.
IF flight-seatsocc = flight-seatsmax.
DELETE available_flights.
ENDIF.
ENDLOOP.
PERFORM let_user_choose_flight USING available_flights
CHANGING choosen_flight.
ENDFORM.
Listing 1
The procedure flight_search executes three tasks:
1. it determines the available flights
2. it calculates a up-to-date-price
3. it let the user choose a flight
If we could extract the determination of the available flights from the other tasks and put this determination in an separate procedure as shown in listing 2, the procedure flight_search would get much cleaner.
FORM fetch_available_flights USING city_to TYPE s_to_city city_from TYPE s_from_cit
begin_date TYPE d end_date TYPE d
CHANGING available_flights TYPE flighttab.
DATA: schedule TYPE STANDARD TABLE OF spfli,
flight TYPE sflight.
SELECT * FROM spfli INTO TABLE schedule
WHERE cityto = city_to AND cityfrom = city_from.
SELECT * FROM sflight INTO TABLE available_flights
FOR ALL ENTRIES IN schedule
WHERE carrid = schedule-carrid AND connid = schedule-connid
AND fldate BETWEEN begin_date AND end_date.
LOOP AT available_flights INTO flight.
IF flight-seatsocc = flight-seatsmax.
DELETE available_flights.
ENDIF.
ENDLOOP.
ENDFORM.
FORM flight_search USING city_to TYPE s_to_city city_from s_cityfrom
begin_date TYPE d end_date TYPE d
CHANGING choosen_flight TYPE sflight.
DATA: available_flights TYPE flighttab.
PERFORM fetch_available_flights USING city_to city_from
begin_date end_date
CHANGING available_flights.
" price is calculated again based on availability
CALL FUNCTION 'ZFLIGHT_PRICE_CALCULATION'
TABLES
flights = available_flights.
PERFORM let_user_choose_flight USING available_flights
CHANGING choosen_flight.
ENDFORM.
Listing 2
But we changed the execution-sequence. The price-calculation in listing 1 was done including fully booked flights. In listing 2 the price calculation is just done with the available flights. How can we verify, that the fully booked flights doesn’t influence the price-calculation of the available flights, if we can’t figure it out from the function-module ZFLIGHT_PRICE_CALCULATION?
The answer is not so difficult as you may think. When we put a breakpoint in listing 1 before the procedure let_user_choose_flight is executed and start debugging and examining the variable available_flights.
Sensing variables
Sensing variables were a idea invented by Micheal Feathers, which help to preserve existing behavior after refactorings. Refactoring in this context means to split a long procedure into smaller procedures.
In this blog-post i want to modify Feathers idea a bit, so that it is more suitable for ABAP developers.
Let’s show an example. The following messy procedure searches flights in a period of time from city A to city B, uses the function-module ZFLIGHT_PRICE_CALCULATION to calculate a up-to-date price and let the user choose one flight:
FORM flight_search USING city_to TYPE s_to_city city_from TYPE s_from_cit
begin_date TYPE d end_date TYPE d
CHANGING choosen_flight TYPE sflight.
DATA: schedule TYPE STANDARD TABLE OF spfli,
available_flights TYPE flighttab,
flight TYPE sflight.
SELECT * FROM spfli INTO TABLE schedule
WHERE cityto = city_to AND cityfrom = city_from.
SELECT * FROM sflight INTO TABLE available_flights
FOR ALL ENTRIES IN schedule
WHERE carrid = schedule-carrid AND connid = schedule-connid
AND fldate BETWEEN begin_date AND end_date.
" price is calculated again based on availability
CALL FUNCTION 'ZFLIGHT_PRICE_CALCULATION'
TABLES
flights = available_flights.
LOOP AT available_flights INTO flight.
IF flight-seatsocc = flight-seatsmax.
DELETE available_flights.
ENDIF.
ENDLOOP.
PERFORM let_user_choose_flight USING available_flights
CHANGING choosen_flight.
ENDFORM.
Listing 1
The procedure flight_search executes three tasks:
1. it determines the available flights
2. it calculates a up-to-date-price
3. it let the user choose a flight
If we could extract the determination of the available flights from the other tasks and put this determination in an separate procedure as shown in listing 2, the procedure flight_search would get much cleaner.
FORM fetch_available_flights USING city_to TYPE s_to_city city_from TYPE s_from_cit
begin_date TYPE d end_date TYPE d
CHANGING available_flights TYPE flighttab.
DATA: schedule TYPE STANDARD TABLE OF spfli,
flight TYPE sflight.
SELECT * FROM spfli INTO TABLE schedule
WHERE cityto = city_to AND cityfrom = city_from.
SELECT * FROM sflight INTO TABLE available_flights
FOR ALL ENTRIES IN schedule
WHERE carrid = schedule-carrid AND connid = schedule-connid
AND fldate BETWEEN begin_date AND end_date.
LOOP AT available_flights INTO flight.
IF flight-seatsocc = flight-seatsmax.
DELETE available_flights.
ENDIF.
ENDLOOP.
ENDFORM.
FORM flight_search USING city_to TYPE s_to_city city_from s_cityfrom
begin_date TYPE d end_date TYPE d
CHANGING choosen_flight TYPE sflight.
DATA: available_flights TYPE flighttab.
PERFORM fetch_available_flights USING city_to city_from
begin_date end_date
CHANGING available_flights.
" price is calculated again based on availability
CALL FUNCTION 'ZFLIGHT_PRICE_CALCULATION'
TABLES
flights = available_flights.
PERFORM let_user_choose_flight USING available_flights
CHANGING choosen_flight.
ENDFORM.
Listing 2
But we changed the execution-sequence. The price-calculation in listing 1 was done including fully booked flights. In listing 2 the price calculation is just done with the available flights. How can we verify, that the fully booked flights doesn’t influence the price-calculation of the available flights, if we can’t figure it out from the function-module ZFLIGHT_PRICE_CALCULATION?
The answer is not so difficult as you may think. When we put a breakpoint in listing 1 before the procedure let_user_choose_flight is executed and start debugging and examining the variable available_flights.
Picture 1
Picture 2
The value of the internal table available_flights is needed for the next steps.
We can either store this value persistent with the tricks shown in my last blog-post or we must remember it otherwise.
In the next step we insert a sensing variable in the procedure flight_search at the position, where we put the breakpoint before, as it’s shown in listing 3.
FORM flight_search USING city_to TYPE s_to_city city_from TYPE s_from_cit
begin_date TYPE d end_date TYPE d
CHANGING choosen_flight TYPE sflight.
DATA: schedule TYPE STANDARD TABLE OF spfli,
available_flights TYPE flighttab,
flight TYPE sflight.
SELECT * FROM spfli INTO TABLE schedule
WHERE cityto = city_to AND cityfrom = city_from.
SELECT * FROM sflight INTO TABLE available_flights
FOR ALL ENTRIES IN schedule
WHERE carrid = schedule-carrid AND connid = schedule-connid
AND fldate BETWEEN begin_date AND end_date.
" price is calculated again based on availability
CALL FUNCTION 'ZFLIGHT_PRICE_CALCULATION'
TABLES
flights = available_flights.
LOOP AT available_flights INTO flight.
IF flight-seatsocc = flight-seatsmax.
DELETE available_flights.
ENDIF.
ENDLOOP.
sensing_available_flights = available_flights.
PERFORM let_user_choose_flight USING available_flights
CHANGING choosen_flight.
ENDFORM.
Listing 3
Out sensing variable sensing_available_variable is defined globally (report-scope, function-group-scope).
Now we prepared everything for an temporary unit-test, that is shown in listing 4.
CLASS test_flight_search DEFINITION FOR TESTING
DURATION SHORT RISK LEVEL HARMLESS.
PRIVATE SECTION.
METHODS no_influence_expected FOR TESTING.
ENDCLASS.
CLASS test_flight_search IMPLEMENTATION.
METHOD no_influence_expected.
DATA: exp_available_flights TYPE flighttab,
choosen_flight TYPE sflight.
" given: The values come from debugging listing 1
exp_available_flights = VALUE #(
( carrid = 'LH' connid = '924' fldate = '20191210' price = 200 seatsocc = 90 seatsmax = 100 )
).
" when
PERFORM flight_search USING 'London' 'Frankfurt' '20191208' '20191212'
CHANGING choosen_flight TYPE sflight.
" then
cl_abap_unit_assert=>assert_equals( exp = exp_available_flights
act = sensing_available_flights ).
ENDMETHOD.
ENDCLASS.
Listing 4
The unit-test cannot to be executed in batch-mode due to the procedure let_user_choose_flight, which interacts with the user.
But if it’s running sucessfull, it proves, that we got the correct value for the sensing variable sensing_available_flights, and with this assurance we can start applying the refactorings from listing 2.
In the additional last step, the when-section in the unit-test can be replaced, so that it omits the interaction with the user and makes the sensing variable sensing_available_flights useless.
CLASS test_flight_search DEFINITION FOR TESTING
DURATION SHORT RISK LEVEL HARMLESS.
PRIVATE SECTION.
" The refactoring from listing 2 should not influence
" the price-calculation
METHODS no_influence_expected FOR TESTING.
ENDCLASS.
CLASS test_flight_search IMPLEMENTATION.
METHOD no_influence_expected.
DATA: exp_available_flights TYPE flighttab,
act_available_flights TYPE flighttab.
" given: The values come from debugging listing 1
exp_available_flights = VALUE #(
( carrid = 'LH' connid = '924' fldate = '20191210' price = 200 seatsocc = 90 seatsmax = 100 )
).
" when
PERFORM fetch_available_flights USING 'London' 'Frankfurt' '20191208' '20191212'
CHANGING act_available_flights.
CALL FUNCTION 'ZFLIGHT_PRICE_CALCULATION'
TABLES
flights = act_available_flights.
" then
cl_abap_unit_assert=>assert_equals( exp = exp_available_flights
act = act_available_flights ).
ENDMETHOD.
ENDCLASS.
Listing 5
Differences to feathers idea
Feathers preferred the determination via unit-tests more than determination with the debugger. But i think, for ABAP with its internal tables and structures determination with the debugger is faster and easier than determination with unit-tests.
No comments:
Post a Comment