This week I needed to refactor some existing code. This step turned out necessary since future developments dealing with this specific part of the code grew more and more complicated. With every new change it took longer and longer to get the expected results.
In addition the outcome was obviously getting more and more fragile, which means, every change I made caused side effects in other parts of the code which I didn’t expect.
All of these signs typically call for a refactoring session, which I started after some weeks of hesitating.
The first goal of the refactoring was to get the source code under control, which means I wanted to apply unit tests as fast as possible to afterwards clamp the behavior of the code by applying various tests.
Once the unit tests would be in place, the change itself could be implemented much safer and more robust. If the unit tests are testing the right things, they would immediately show, what went wrong with one of the last changes.
But applying unit tests to existing code which had no unit tests before is often not easy.
The reason for this is dependencies. To give you an example: Every time you use validation logic around database calls, you couple dependencies too close, which means you move far away from a testable architecture.
Consider this example:
METHOD is_open. DATA lv_status TYPE z_doc_status. SELECT SINGLE status FROM z_doc INTO lv_status WHERE id = mv_id. IF lv_status = ‘O’ OR lv_status = space. rv_is_open = abap_true. ELSE. rv_is_open = abap_false. ENDIF. ENDMETHOD.
To test such a method in a unit test, you would have to insert a corresponding record to the database before, in order to have something to select from within the method implementation.
This method should be refactored in order to apply a unit test. One possibility is to define its own class for accessing database entries. This kind of class is called “repository”.
With an existing repository, the method could look like this:
METHOD is_open. DATA lv_status TYPE z_doc_status. lv_status = mo_doc_repository->get_status( mv_id ). IF lv_status = ‘O’ OR lv_status = space. rv_is_open = abap_true. ELSE. rv_is_open = abap_false. ENDIF. ENDMETHOD.
The repository now takes care of selecting what we are asking for. In Unit Tests, you could fake this repository by using a local class implementation which returns hard coded values instead of database calls. This makes testing easier, more robust and repeatable.
However, we didn’t have a unit test yet. This step was just the preparation to apply a unit test to this method. How do we know, that the code didn’t already break yet?
Using a vise to ensure that nothing broke
The concept of a vise is something I didn’t invent. I just read Michael Feathers blog posts which dealt with the question how one can ensure that nothing broke during one of the last changes in source code.
The concept is quite easy: Before a change to the code, you record the content of a freely chosen variable. After the change, you ensure that the value stays the same.
With the vise implementation I wrote for ABAP, capturing the value of a specific variable is quite simple: Just call ZCL_VISE=>GRIP( … ) like in this example:
METHOD is_open. DATA lv_status TYPE z_doc_status. SELECT SINGLE status FROM z_doc INTO lv_status WHERE id = mv_id. ZCL_VISE=>GRIP( lv_status) IF lv_status = ‘O’ OR lv_status = space. rv_is_open = abap_true. ELSE. rv_is_open = abap_false. ENDIF. ENDMETHOD.
Running this code for the first time causes Vise to write the contents of the status value to the database. By the way, this also works, if you perform a ROLLBACK WORK later on.
Running the coding after the change should not cause Vise to throw an exception. But if the value of lv_status had changed, it would. Of course you should make sure to process the same entity, which means the same document ID, in order to get the same status.
To run the code, you could either apply a unit test with a hard-coded document ID or just run it using an application UI (which is yet another form of a test, although not the same like a fine grained unit test)
METHOD is_open. DATA lv_status TYPE z_doc_status. lv_status = mo_doc_repository->get_status( mv_id ). ZCL_VISE=>GRIP( lv_status) IF lv_status = ‘O’ OR lv_status = space. rv_is_open = abap_true. ELSE. rv_is_open = abap_false. ENDIF. ENDMETHOD.
After you figured out, that nothing has changed and the behavior of the code stayed the same, you can apply tests. As I mentioned, applying unit tests fixes the specification against your application behavior and makes future changes to the code easier and safer, hence less costly.
Do not forget to remove the calls to Vise after the changes. As a development tool, Vise is not intended to be shipped within productive code!
Specific considerations on ABAP
In ABAP you may have work areas and internal tables, besides objects and primitive data types.
Vise can track also the contents of internal tables and work areas. In addition it allows you to skip certain fields when validation against previous recordings is done.
All you have to do is to specify the components which are to be ignored. You can pass this information with the GRIP( ) – call:
DATA lt_ignore TYPE string_table. APPEND ‘PRICE’ to lt_ignore. ZCL_VISE=>GRIP( i_data = lt_sflight it_ignore = lt_ignore ).
Tracking object contents is not yet supported. Feel free to join the project and implement such a comparison e.g. for all public get-methods and attributes.
Where to get Vise
Vise is currently hosted on Github
Vise for Java http://www.artima.com/weblogs/viewpost.jsp?thread=171323
How to write trustworthy and effective unit tests in general http://www.manning.com/osherove/
How to bring legacy code under test coverage http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052/