Application Development Blog Posts
Learn and share on deeper, cross technology development topics such as integration and connectivity, automation, cloud extensibility, developing at scale, and security.
cancel
Showing results for 
Search instead for 
Did you mean: 
joltdx
Active Contributor
Do your write code only for machines, or also for your fellow developers and curious functional consultants?

I believe that readability of program code is an important long-term aspect. I try to have it as a big priority and I'm constantly trying to improve my ability to use naming and composition to make the code as self-explanatory as possible. Comments still have their place though, but should not be what the code is based upon.

The online Clean ABAP style guides has a good section on comments, as does the new Clean ABAP book.

I recommend both those sources with all my heart, but let me elaborate here as well how we can write code for humans and machines at the same time...

Four quadrants of code and comments


So if we take for one aspect the naming of things in our code, like variables, classes, methods, etc. We define a very simple 2 grade scale grading how well the naming explains the thing, from 'Nope' to 'Very much so'.

For another aspect, we take the degree to which comments are used to explain the coding. Again with the same 2 grade scale.

Let's put this in four quadrants for a simplified world.


Four quadrants of code and comments


Let's get to it then...

 

1. Nope - Nope


First example is really really common - no comments and no good naming. Probably work ok right when you type it but even after a week, you yourself might not have a clue what your were up to:
lr_tma = go_asmnt->todo( ).
ln_pt = 2.
lr_tma->find_impnt( ln_pt ).
WHILE NOT lr_tma->complete( ln_pt ).
go_asmnt->fn_main( lr_tma ).
ENDWHILE.

Hey, I made this up myself and I'm not sure I get it... 😄

 

2. Very much so - Nope


All right, so clearly we need to improve on that one... Lets add comments to explain the code:
" Get pending tasks for assignment
lr_tma = go_asmnt->todo( ).

" Sort the tasks in order of priority (sort type = 2)
ln_pt = 2.
lr_tma->find_impnt( ln_pt ).

" Execute all of the tasks, one at a time, in order
WHILE NOT lr_tma->complete( ln_pt ).
" Execute
go_asmnt->fn_main( lr_tma ).
ENDWHILE.

Now, finding this code 6 months later, at least we know where to look for maintenance and what it's supposed to do... It's much better...

But are we there yet? No, there are still better ways in my subjective opinion...

 

3. Very much so - Very much so


Now, if we instead fix it up a bit and rename our variables, objects and methods...
" Get pending tasks for assignments
tasks = assignments->get_pending_tasks( ).

" Sort the tasks in order of priority
tasks->sort_by_priority( ).

" Execute all of the tasks, one at a time, in order
WHILE NOT tasks->is_empty( ).
" Execute
assignments->execute_next( tasks ).
ENDWHILE.

Now the naming is explaining the code and the comments are explaining the code. Ok, all good? Twice as good? Well. Yes but no. Since the coding basically says the same thing as the comments now, we need to read the same thing twice... And it also has to be written twice... And changed twice when making adjustments or adding features...

And what if the comment and the code is not saying the same thing? Do we trust the comment or the code?

Let's carry on to the final one and my personal favorite...

 

4. Nope - Very much so


tasks = assignments->get_pending_tasks( ).
tasks->sort_by_priority( ).

WHILE NOT tasks->is_empty( ).
assignments->execute_next( tasks ).
ENDWHILE.

Ah! The cleanliness. Quick to read. Quick to get an overview. Understandable code.

Just as Jules is trying real hard to be the shepherd in Pulp Fiction, I try real hard to write this kind of code. I'm not where I want to be yet, but I'm trying real hard...

 

5. What, that's it? Just like that? No comments ever?


Well no. First, there are a couple of important prerequisites for this... One prerequisite is a good composition of the code, the division of code into clearly defined and scoped classes and methods. If your code is 1500 lines in a procedural style, then naming things well certainly help, but you still risk getting lost anyway.

And a prerequisite to do that well is to use ABAP OO, which we all should whenever possible!

Another is context. I mean in the above example there are still many things we can't say other than that this code is supposed to be prioritizing and executing. Hopefully it's clear from the context where the code is located what an assignment is and what kind of tasks we're talking about, for instance.

Comments are still a very good way to explain WHY something is done in a certain way, that might not be obvious... Something like this, for example:
" For some reason, the BAPI_CREATE_SOMETHING is not accepting item_id
" to be '0001' when creating a new object and assigning items at the
" same time. We use '1337' to prepare the data instead. This is fine.
items = VALUE #( ( item_id = '1337'
description = some_data->title
value = some_data->value ) ).

(It's funny how the styling of the code blocks here are syntax highlighting inside the comments...)

 

Now, if only the reality was this simple... 🙂

Stay safe everybody and let's hope to understand each others code...

Addition: To keep reading on this topic in the SAP Community, I recommend continuing with I See Dead Code after getting the extra value from the comments below.
33 Comments
Michael_Keller
Active Contributor
My favorite subject 🙂 So much can be achieved with little effort if you choose names appropriately.Although choosing the right name sometimes takes a long time. At least for me. But it's worth it.

I cannot live completly without comments in my source code. I don't want that either. But I know that a comment is really important when I see it 😉

By the way: I work a lot with the ABAP Development Tools. If you write methods in it, the short text is missing in the SE24 and also in the ADT via F2 (element information). That's why I now use ABAP Doc to give some methods a synchronized english short text. But only short text and it must fit in one line 🙂
larshp
Active Contributor
Exactly, as I typically joke "there should be no comments in the code", above illustrates this perfectly 🙂
joltdx
Active Contributor
Yes, simple isn't easy... 🙂

Good point - save the comments for when they're needed. "Don't cry wolf", that's a global saying, right?
joltdx
Active Contributor
I've seen cases where I wonder if there's any code among the comments... At least the intention there was right, though...
TimoStark
Participant

Very well written. Unfortunately with comment to code relation checks the ‘what is the following code doing’ (or worse method history with change log.. Brrrr…) comment style is enforced. Let’s hope for the day where the last one was convinced that this measure doesn’t make any sense 🙂

joltdx
Active Contributor
0 Kudos
Thanks. Yes let's work on having that day come sooner!
nomssi
Active Contributor
My comment to your code without comment would be "ante up, use an iterator":
tasks = assignment->new_iterator( ).
WHILE tasks->has_next( ).
assignment->execute( tasks->next( ) ).
ENDWHILE.

Even in this code, I would like to add a comment asking if assignment is really the right object to be responsible for executing a task. So maybe I would add a dimension "comment explaining the abstraction".

But I like your quadrants simplification very much.

regards,

JNN
joltdx
Active Contributor
0 Kudos
Yes, Jacques, you have a perfect case-in-point here, thank you! Context is really necessary for understanding the code. Depending of what assignment is and what tasks are, your implementation with the iterator pattern might be the perfect solution. And assignment and tasks may be totally wrong names. 🙂

 
BaerbelWinkler
Active Contributor
Interesting discussion, thanks for starting it Jörgen!

I like to put comments into code (and may be overdoing it sometimes!), mostly to explain why something is done. Here are two examples from a program I recently wrote utilizing the RTTS-framework for the first time.


and


Without comments like these, I wouldn't remember for long what and why I did there!

Note: I had copied the RTTS-logic from one of the DEMO-programs which used prefixes and form-routines and as that already required some "wrapping my head around" about what's going on there, I didn't dare try to convert this to ABAP-OO and mess with the names used. And, I wasn't even sure if ABAP-OO would have been an option for RTTS given the dynamic nature of the structures used.

Cheers

Bärbel
joltdx
Active Contributor
0 Kudos
Thank you for the real world actual examples.

I totally agree with the why-comments... And I really like you also adding the links in there for more information, to give that extra added value.

I had only two axis in the simplified worldview I described, but I guess another one could be "complexity" or something. Like "Is this understood by the majority of ABAPers or do I need to explain this command or what I'm doing here?". For instance, many developer are not used to the VALUE operator yet, but I think that one is pretty straight forward. Just a F1 away. But I guess it can be easy to get carried away and writing an overly complex statement using the REDUCE operator...

 
matt
Active Contributor

I see a comment before a block of code as an indicator that perhaps the code should be in its own method. I generally try to do that whenever it turns up. It’s no always quite so easy, so with established code, sometimes I just have to leave it.

I do put comment lines above SE91 messages used in code with the text. It's handy to have some indication. Of course that leaves the possibility that the message will change and the code won't... 

 

 

matt
Active Contributor
With your example here, (notwithstanding your caveats), I would have modularised like this:
IF p_compr EQ abap_true.
do_stuff_with_meaningful_name_1.
ENDIF.

" do_stuff_with_meaningful_name_1
READ TABLE...
IF sy-subrc EQ 0.
do_stuff_with_meaningful_name_2.
ENDIF.

" do_stuff_with_meaningful_name_2
DATA...
WHILE...
do_stuff_with_meaningful_name_3.
exit loop if done.
ENDWHILE.

" do_stuff_with_meaningful_name_3
role_and_user...

And then adjusted the WHY comments accordingly.
matt
Active Contributor
0 Kudos
I remember being told I had to comment a bit of code not much more complex than this.
METHOD read_data.
READ TABLE some_table WITH TABLE KEY field = i_value.
IF not_found( ).
MESSAGE 'Not there' TYPE 'I' DISPLAY LIKE 'E'.
ELSE.
MESSAGE 'It is there' TYPE 'I' DISPLAY LIKE 'S'.
ENDIF:
ENDMETHOD.

METHOD not_found.
r_result = xsdboolc( sy-subrc IS NOT initial ).
ENDMETHOD.

I asked "what comment would you like me to put in"?

Change logs -> yes, they became part of the standard in my last year at my previous place of work. I'm glad I left.
Jelena
Active Contributor
This seems like a nice opportunity to pimp my not so old blog I See Dead Code. The "Begin of changes... end of changes" fall somewhere near 0 in your quadrant. 🙂

"Narration" of the code is my second pet peeve ("*Here I'm selecting data from database" followed by SELECT). This argument: "the comment and the code is not saying the same thing" should, hopefully, hit the last nail in its coffin.
Jelena
Active Contributor
0 Kudos
Can relate. I ran into this requirement with the ATC check in one project: code must have X comments per X lines or something. Honestly, I don't understand why this is necessary and what is the logic behind it. Sometimes short code may need a comment and sometimes longer code is self-explanatory. Whether to comment or not has nothing to do with the code size. (I have yet to see any research that would find otherwise.)

I wish more SAP customers would look into practicality and "cleanliness" of their ATC checks instead of just blindly adopting something. Otherwise it defeats the purpose of having any quality check.
joltdx
Active Contributor
"Program is a living organism and dead code is as useful as a gangrene. Let it rest in peace in the version control."

Love it!

I'll actually go ahead and add a link to your post above, in case somebody misses these comments... But I hope nobody does, as there is great extra value in the comments both here and in your post. Thanks!
joltdx
Active Contributor
0 Kudos
There is a specific rule against that in the clean code guidelines, but I agree with you on this one (as well). It's true that the message details and phrasing might change. But I consider those as explaining the why rather than indicating the exact error message text. "Ok, so we're issuing a MESSAGE here, why is that?". It's not always obvious for instance if no match from reading a table is totally fine or if it's really really bad.

And if messages are repurposed to mean a totally different thing later on, well, then there are bigger problems... 🙂
igor_simonovic
Explorer
Maybe I would use this:

 

try.

assignments->execute_pending_tasks( im_sort = by_priority ).

catch zcx_execution_exc.

endtry.

 

If the code is well organized and named then comments can become easily just a duplicate of code and then I would rather omit them. Sometimes it is not easy to create such a code and some help from comments is needed.  One problem with SAP I see is that I have only 30 chars long names, which is short if we take into account that we need to use some prefix also (like ZCL_ ...here 4 chars are lost...and there are even teams which use much longer prefixes because of their local dev standards and it can happen that you have only 20 chars remaining for some meaningful name). One reason why it is difficult to create meaningful names is that this require much of thinking to create good design. It is close to impossible to find a meaningful name if ta code design is low quality.
joltdx
Active Contributor
Yes, it's not easy...

And even harder is naming database tables... 16 characters is not much in the vast open global space where they all share space...
Föß
Active Participant
Hi,

I agree to 100%. - I am using Regex very often. Regex is a great example where we need comments for better understanding.

Thx, Johann
joltdx
Active Contributor
0 Kudos
Hah! Yes, it doesn't take many characters for a regular expression to be hard to understand.

Agreed on the necessity to comment that one if it's not encapsulated in a super well named single method, which will probably never be the case.

Out of curiosity, do you tend do document only what it does, or also the explanation like
" ^(\d*)HELLO
" ^ - This is the start of the string
" (\d*) - First subgroup
" \d - digits
" * - any number of, in this case, digits
" HELLO - the literal string 'HELLO'

I mean I see the benefit of the explanation but for a longer string, and if in need of maintenance, how about this? 😄
" Just paste this regular expression into regex101.com.
" You will get the explanation and have the possibility to test it
igor_simonovic
Explorer
Jörgen, thanks for info about    regex101.com.  I pasted the regex above there and it provided nice explanation.
vinay_udekar
Explorer
Oh I liked the four quadrant approach. Never thought about it this way and it's so easy to visualize and use it. I think much of this variable naming and commenting discipline starting with when we start learning the programming language. I remember the times in my QA reviews when naming a variable as l_count meant that we are counting something and then invariably getting into l_count1, l_count2 and so on often forgetting what that count was meant for. Same for l_flag. I also think that on occasions where our only requirement is do a bug fix in some incident management engagement and our scope is maybe writing 10-15 lines of code, the programmer should make an attempt to write them in as clean manner as possible ignoring how the remaining program was written.

Couldn't agree more on the "WHY something is written" instead of the narrative.

Thanks for sharing your thoughts
joltdx
Active Contributor
0 Kudos
Yes, old habits die hard. And if we don't think about it, we'll probably just keep going...

Thanks for your thoughts!
JozsefSzikszai
Active Contributor
Fully agree about choosing a meaningful class/method name, however the 30 character limit (minus prefixes and underscores) is usually an issue.

 

On the side note, I still see lots of "comments" like this:
* Sorting the internal table
SORT itab.
joltdx
Active Contributor
Yes, and even harder is naming database tables in max 16 characters...
JozsefSzikszai
Active Contributor
On one of my previous projects there was a 14 character prefix for DB tables. I had to rename a DB  table twice, because the client said, I was not able to find a meaningful name for the remaining 2 characters 🙂
Jelena
Active Contributor
I find that teams that require long prefixes need to be told that sub-packages exist in SAP. Other than ZCL... or ZCX... that are mandated by SAP there really shouldn't be anything else required.
Patrick_vN
Active Contributor
Don't forget that a lot of companies have coding conventions & guidelines. Some are more outdated as others, but in my experience, there is no easy way to convince those customers to change the guidelines..

So often the team has not a lot of room to maneuver, and often more important battles to fight. Little that can be done about that I guess.
shais
Participant
Since object namespace isn't available in ABAP repository, many times adding a (short) prefix is inevitable in order to avoid of a complete mess.
Jelena
Active Contributor
0 Kudos
Can relate to that. 🙂
Jelena
Active Contributor
0 Kudos
Good point, thanks!
BaerbelWinkler
Active Contributor
Here is an example for a comment where I really would have appreciated some clues as to "Why"?!?
tables: ZEHS_ITAB_NEU, pruefwerte. " don't remove!

ZEHS_ITAB_NEU is a dictionary defined structure but a where-used didn't turn up anything apart from the code I was in, namely a form-routine in an enhancement related to delivery processing (VL01/02/03). For the change I needed to apply, a new field was needed in this structure which for various reasons I couldn't quickly do right away in the dictionary. As there was no explanation for the "don't remove!" comment, I tried with putting the structure directly into the form routine as a local data definition. This at least didn't cause any syntax errors but it caused some information on the delivery note to go AWOL, namely some long texts. Digging deeper, it turned out that the structure was used in SO10-long text definitions with some SAPScript logic thrown in for good measure. This logic only works with the structure being a global work area defined via the TABLES-statement.

Would have been nice to see this spelled out in a comment (I think)!