ABAP Maintainability – Why commenting out code lines is not a good idea
When looking at older ABAP programs, which I quite often do for analysis and maintenance purposes, it often strikes me that a large number of code lines have been ‘commented out’: an asterix (*) has been placed before the original code. Every ABAP-programmer knows code like the (fictitious) example below: old code has been replace by new lines to improve the code or change its functionality.
* BEGIN OF DELETION BME20130101
*SELECT * FROM vbak INTO order_header WHERE vbeln IN so_vbeln.
* SELECT * FROM vbap INTO order_line WHERE vbeln = order_header-vbeln.
* “here, some fancy stuff happens.
* END OF DELETION
* BEGIN OF INSERTION BME20130101
SELECT * FROM vbak INTO TABLE order_headers WHERE vbeln IN so_vbeln.
IF lines( order_headers ) > 0.
SELECT * FROM vbap INTO TABLE order_lines FOR ALL ENTRIES IN order_headers
WHERE vbeln = order_headers–vbeln.
LOOP AT order_headers INTO order_header.
LOOP AT order_lines INTO order_line WHERE vbeln = order_header–vbeln.
“here, some fancy stuff happens.
* END OF INSERTION
This practice is often advocated or even made obligatory in development guidelines. When asked why, I often get one or more of the following reasons:
- To improve maintainability of the code. Using this practice, other programmers are better able to track changes to code and thus analyze possible errors.
- It’s easier to revert to older code versions.
- I still remember what I was doing the next day and If I get sick, other people can see what I was doing.
- It was a practice in my previous job as an ….programmer where we didn’t have any source control tools.
I consider commenting out ABAP code (or for that matter, any code) a bad practice because 1) it affects readability and thus maintainability in a negative way, and 2) you don’t need it because you can use version management:
- The code gets clogged by old code that is not relevant anymore, which seriously affects the readability and thus the maintainability of the ABAP code. As someone puts it “sometimes it’s like excavating a tomb to find the actual code that is still being used.“ (http://stackoverflow.com/questions/123423/do-you-leave-historical-code-commented-out-in-classes-that-you-update)
- SAP offers Version Management for most workbench objects. (why SMARTforms has been excluded from version management still beats me 🙁 .) It is possible to compare versions on the same system or on another connected SAP system. This way, you can always check the version that is running in production. It is also possible to revert to an older version or to store an intermediate version. To be sure, version management in SE80 could be improved by offering better tools to merge different versions, but I expect that ABAP in Eclipse offers much better options for that. I still have to check that out.
Commenting out code only makes sense sometimes during developments, when your not exactly sure if your changes are correct. Then these comments can help you to quickly understand what is happening, why your changes don’t work as expected etc. However, when your code is fully tested and ready for transporting it into production, take a few minutes extra to delete all the code that was commented out. And while doing that, maybe it’s also a good idea to delete unused variables and even routines or methods.
Should the code have no information at all about the changes that were implemented? No, that’s not what I think. In my opinion, adding a short, to-the-point description of an implemented change in the header of a program, a function pool, a class pool, preferably with a link to issue tracking identification and other types of documentation, is worth considering, because it helps you and subsequent programmers to understand the current functionality and technical design of your code. Just make sure that this inline documentation, just as all the other documentation is relevant, up-to-date and correct. But that is a subject for another blog.